Re: [nfs-utils PATCH] statd: fix notify_list struct leak in nlist_free

[Steve Dickson <steved@redhat.com>]

On 9/3/26 1:50 PM, Scott Mayhew wrote:
> nlist_free() freed an entry's string members but never the notify_list
> struct itself, despite its name and comment ("Destroy an entry ... and
> free the memory"). Every caller was expected to free the struct
> separately, but several did not:
> 
>    - process_reply() and process_notify_list() (rmtcall.c) clean up
>      entries cloned onto the notify list during SM_NOTIFY handling.
>    - sm_unmon_1_svc() and sm_unmon_all_1_svc() (monitor.c) leak the same
>      way on the SM_UNMON paths.
> 
> Fix the root cause by having nlist_free() free the struct, and drop the
> now-redundant free() calls in sm_mon_1_svc() and load_one_host() that
> would otherwise double-free.
> 
> nlist_kill() previously called nlist_free(head, *head) followed by
> free(*head); since nlist_remove() (called from nlist_free) already
> advanced *head to the next entry, that free(*head) freed the wrong,
> still-live element. Pass NULL so nlist_free() only frees the entry and
> advance the list manually.
> 
> Assisted-by: Claude Opus 4.8 <noreply@anthropic.com>
> Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Committed... (tag: nfs-utils-2-9-3-rc4)

steved.
> ---
>   utils/statd/monitor.c | 2 --
>   utils/statd/notlist.c | 4 ++--
>   2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
> index 76ef16f1..f7f4342a 100644
> --- a/utils/statd/monitor.c
> +++ b/utils/statd/monitor.c
> @@ -225,7 +225,6 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
>   
>   failure:
>   	xlog_warn("STAT_FAIL to %s for SM_MON of %s", my_name, mon_name);
> -	free(clnt);
>   	return (&result);
>   }
>   
> @@ -245,7 +244,6 @@ load_one_host(const char *hostname,
>   	clnt->dns_name = strdup(hostname);
>   	if (clnt->dns_name == NULL) {
>   		nlist_free(NULL, clnt);
> -		free(clnt);
>   		return 0;
>   	}
>   
> diff --git a/utils/statd/notlist.c b/utils/statd/notlist.c
> index 45879a43..91030d85 100644
> --- a/utils/statd/notlist.c
> +++ b/utils/statd/notlist.c
> @@ -210,6 +210,7 @@ nlist_free(notify_list **head, notify_list *entry)
>   	if (NL_MON_NAME(entry))
>   		free(NL_MON_NAME(entry));
>   	free(entry->dns_name);
> +	free(entry);
>   }
>   
>   /*
> @@ -222,8 +223,7 @@ nlist_kill(notify_list **head)
>   
>   	while (*head) {
>   		next = (*head)->next;
> -		nlist_free(head, *head);
> -		free(*head);
> +		nlist_free(NULL, *head);
>   		*head = next;
>   	}
>   }