Re: [PATCH 3/4] nscd: Avoid null pointer crashes after notfound response (bug 31678)

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2024-04-24 12:08, Florian Weimer wrote:
> The addgetnetgrentX call in addinnetgrX may have failed to produce
> a result, so the result variable in addinnetgrX can be NULL.
> Use db->negtimeout as the fallback value if there is no result data;
> the timeout is also overwritten below.
> 
> Also avoid sending a second not-found response.  (The client
> disconnects after receiving the first response, so the data stream did
> not go out of sync even without this fix.)  It is still beneficial to
> add the negative response to the mapping, so that the client can get
> it from there in the future, instead of going through the socket.
> ---

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

>   nscd/netgroupcache.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> index 8709fb77b6..8f9eb84e39 100644
> --- a/nscd/netgroupcache.c
> +++ b/nscd/netgroupcache.c
> @@ -509,14 +509,15 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req,
>   
>     datahead_init_pos (&dataset->head, sizeof (*dataset) + req->key_len,
>   		     sizeof (innetgroup_response_header),
> -		     he == NULL ? 0 : dh->nreloads + 1, result->head.ttl);
> +		     he == NULL ? 0 : dh->nreloads + 1,
> +		     result == NULL ? db->negtimeout : result->head.ttl);
>     /* Set the notfound status and timeout based on the result from
>        getnetgrent.  */
> -  dataset->head.notfound = result->head.notfound;
> +  dataset->head.notfound = result == NULL || result->head.notfound;
>     dataset->head.timeout = timeout;
>   
>     dataset->resp.version = NSCD_VERSION;
> -  dataset->resp.found = result->resp.found;
> +  dataset->resp.found = result != NULL && result->resp.found;
>     /* Until we find a matching entry the result is 0.  */
>     dataset->resp.result = 0;
>   
> @@ -564,7 +565,9 @@ addinnetgrX (struct database_dyn *db, int fd, request_header *req,
>         goto out;
>       }
>   
> -  if (he == NULL)
> +  /* addgetnetgrentX may have already sent a notfound response.  Do
> +     not send another one.  */
> +  if (he == NULL && dataset->resp.found)
>       {
>         /* We write the dataset before inserting it to the database
>   	 since while inserting this thread might block and so would