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

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

[Scott Mayhew]

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>
---
 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;
 	}
 }
-- 
2.55.0

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

[Steve Dickson]

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;
>   	}
>   }