This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ #16365] Fix infinite loop in nscd when netgroup is empty
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>, libc-alpha at sourceware dot org
- Date: Wed, 01 Jan 2014 22:38:58 -0500
- Subject: Re: [PATCH][BZ #16365] Fix infinite loop in nscd when netgroup is empty
- Authentication-results: sourceware.org; auth=none
- References: <20131223115002 dot GH4979 at spoyarek dot pnq dot redhat dot com> <20131231064550 dot GB5374 at spoyarek dot pnq dot redhat dot com>
On 12/31/2013 01:45 AM, Siddhesh Poyarekar wrote:
> Ping!
>
> On Mon, Dec 23, 2013 at 05:20:02PM +0530, Siddhesh Poyarekar wrote:
>> Hi,
>>
>> Currently, when a user looks up a netgroup that does not have any
>> members, nscd goes into an infinite loop trying to find members in the
>> group. This is because it does not handle cases when getnetgrent
>> returns an NSS_STATUS_NOTFOUND (which is what it does on empty group).
>> Fixed to handle this in the same way as NSS_STATUS_RETURN, similar to
>> what getgrent does by itself.
>>
>> Tested on Fedora rawhide to verify that the problem is fixed. OK to
>> commit?
>>
>> Siddhesh
>>
>> [BZ # 16365]
>> * nscd/netgroupcache.c (addgetnetgrentX): Break if status is
>> NSS_STATUS_NOTFOUND.
>>
>> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
>> index a607dda..f555892 100644
>> --- a/nscd/netgroupcache.c
>> +++ b/nscd/netgroupcache.c
>> @@ -180,9 +180,10 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
>> int e;
>> status = getfct.f (&data, buffer + buffilled,
>> buflen - buffilled, &e);
>> - if (status == NSS_STATUS_RETURN)
>> - /* This was the last one for this group. Look
>> - at next group if available. */
>> + if (status == NSS_STATUS_RETURN
>> + || status == NSS_STATUS_NOTFOUND)
>> + /* This was either the last one for this group or the
>> + group was empty. Look at next group if available. */
>> break;
>> if (status == NSS_STATUS_SUCCESS)
>> {
OK.
It is silly that the current function that implements `f' can
clearly return NSS_STATUS_NOTFOUND and yet the caller doesn't
test for this condition.
I have also reviewed that these are two return codes are the
only useful ones in this case. I don't think we need to test
for any others.
Cheers,
Carlos.