Re: [bz 22342] patch V2: fix netgroup cache keys

[Carlos O'Donell <carlos at redhat dot com>]

On 03/01/2018 06:28 PM, DJ Delorie wrote:
> 
> Unlike other nscd caches, the netgroup cache contains two types of
> records - those for "iterate through a netgroup" (i.e. setnetgrent())
> and those for "is this user in this netgroup" (i.e. innetgr()),
> i.e. full and partial records.  The timeout code assumes these records
> have the same key for the group name, so that the collection of records
> that is "this netgroup" can be expired as a unit.
> 
> However, the keys are not the same, as the in-netgroup key is generated
> by nscd rather than being passed to it from elsewhere, and is generated
> without the trailing NUL.  All other keys have the trailing NUL, and as
> noted in the linked BZ, debug statements confirm that two keys for the
> same netgroup are added to the cache with two different lengths.
> 
> The result of this is that as records in the cache expire, the purge
> code only cleans out one of the two types of entries, resulting in
> stale, possibly incorrect, and possibly inconsistent cache data.
> 
> The patch simply includes the existing NUL in the computation for the
> key length ('key' points to the char after the NUL, and 'group' to the
> first char of the group, so 'key-group' includes the first char to the
> NUL, inclusive).

Perfect. Please commit with this explanation as the commit message.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> 	[BZ #22342]
>  	* nscd/netgroupcache.c (addinnetgrX): Include trailing NUL in
>  	key value.
> 
> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> index b832c9315f..2f187b208c 100644
> --- a/nscd/netgroupcache.c
> +++ b/nscd/netgroupcache.c
> @@ -480,7 +480,7 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req,
>  {
>    const char *group = key;
>    key = (char *) rawmemchr (key, '\0') + 1;
> -  size_t group_len = key - group - 1;
> +  size_t group_len = key - group;
>    const char *host = *key++ ? key : NULL;
>    if (host != NULL)
>      key = (char *) rawmemchr (key, '\0') + 1;
> 


-- 
Cheers,
Carlos.