This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] CVE-2014-8121: Fix nss_files file management [BZ#18007]
- From: Florian Weimer <fweimer at redhat dot com>
- To: schwab at suse dot de
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 01 Apr 2015 19:41:20 +0200
- Subject: Re: [PATCH] CVE-2014-8121: Fix nss_files file management [BZ#18007]
- Authentication-results: sourceware.org; auth=none
- References: <54EB120A dot 1010202 at redhat dot com> <5506F010 dot 1090608 at redhat dot com> <mvmlhil1n5g dot fsf at hawking dot suse dot de> <871tkdtg89 dot fsf at mid dot deneb dot enyo dot de> <mvmbnjh1c3v dot fsf at hawking dot suse dot de> <87wq25s0dh dot fsf at mid dot deneb dot enyo dot de> <mvm7fu51b3r dot fsf at hawking dot suse dot de> <87r3sdrzo4 dot fsf at mid dot deneb dot enyo dot de> <mvmpp7xytjt dot fsf at hawking dot suse dot de>
On 03/25/2015 04:40 PM, Andreas Schwab wrote:
> You need to take a global view at the bug. The sharing of global state
> between getXXent and getXXbyYY is broken by design.
I don't feel qualified to review this. It looks like reasonable change.
It does fix bug 18007 (and more). It does not change the fopen/fclose
behavior of getent in my simple tests (for nss_files); the file is not
kept open before and after the patch.
> diff --git a/nis/nss_compat/compat-grp.c b/nis/nss_compat/compat-grp.c
> index 5e9c527..987d914 100644
> --- a/nis/nss_compat/compat-grp.c
> +++ b/nis/nss_compat/compat-grp.c
> static enum nss_status
> -internal_endgrent (ent_t *ent)
> +internal_endgrent (ent_t *ent, int needent)
> {
> - if (nss_endgrent)
> + if (needent && nss_endgrent)
> nss_endgrent ();
I think the nss_endgrent call should be moved into _nss_compat_endgrent,
where it belongs. Then the needent argument is not necessary. This
applies to the other files as well.
> diff --git a/nss/nss_files/files-XXX.c b/nss/nss_files/files-XXX.c
> index a7a45e5..aabd4ea 100644
> --- a/nss/nss_files/files-XXX.c
> +++ b/nss/nss_files/files-XXX.c
> @@ -63,21 +63,18 @@ __libc_lock_define_initialized (static, lock)
> /* Maintenance of the shared stream open on the database file. */
This comment is now outdated, and
/* Locks the static variables in this file. */
should be updated as well (the lock only covers the “file” variable).
Similarly for the parallel places in the other files.
--
Florian Weimer / Red Hat Product Security