Re: [PATCH net-next v3 1/3] af_unix: Schedule the garbage collector at task exit

[Paolo Abeni <pabeni@redhat.com>]

On 7/22/26 11:31 AM, Nam Cao wrote:
> When a task exits while still having dead cyclic reference AF_UNIX sockets,
> those sockets stay behind indefinitely until the garbage collector gets
> scheduled by an unrelated reason. This can be observed with the program
> below.
> 
> Resolve this issue by scheduling the garbage collector during task exit,
> after the task's file descriptors have been closed.
> 
>  #include <sys/mount.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
>  #include <sys/wait.h>
> 
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <unistd.h>
> 
> static int send_fd(int unix_fd, int fd)
> {
>         struct msghdr msgh;
>         struct cmsghdr *cmsg;
>         char buf[CMSG_SPACE(sizeof(fd))];
> 
>         memset(&msgh, 0, sizeof(msgh));
> 
>         memset(buf, 0, sizeof(buf));
>         msgh.msg_control = buf;
>         msgh.msg_controllen = sizeof(buf);
> 
>         cmsg = CMSG_FIRSTHDR(&msgh);
>         cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
>         cmsg->cmsg_level = SOL_SOCKET;
>         cmsg->cmsg_type = SCM_RIGHTS;
> 
>         msgh.msg_controllen = cmsg->cmsg_len;
> 
>         memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
>         return sendmsg(unix_fd, &msgh, 0);
> }
> 
> int main(int argc, char *argv[])
> {
> 	int fd[2];
> 	int i;
> 
> 	for (int n = 0; n < 100; ++n) {
> 		if (socketpair(PF_UNIX, SOCK_SEQPACKET, 0, fd) == -1)
> 			goto out_error;
> 
> 		for (i = 0; i < 100; ++i) {
> 			if (send_fd(fd[0], fd[0]) == -1)
> 				goto out_error;
> 
> 			if (send_fd(fd[1], fd[1]) == -1)
> 				goto out_error;
> 		}
> 	}
> 
> 	return 0;
> 
> out_error:
> 	fprintf(stderr, "error: %s\n", strerror(errno));
> }
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
>  include/net/af_unix.h | 5 +++++
>  kernel/exit.c         | 7 +++++++
>  net/unix/af_unix.h    | 1 -
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 34f53dde65ce..686f6f1d1c21 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -14,11 +14,16 @@
>  
>  #if IS_ENABLED(CONFIG_UNIX)
>  struct unix_sock *unix_get_socket(struct file *filp);
> +void unix_schedule_gc(struct user_struct *user);
>  #else
>  static inline struct unix_sock *unix_get_socket(struct file *filp)
>  {
>  	return NULL;
>  }
> +
> +static inline void unix_schedule_gc(struct user_struct *user)
> +{
> +}
>  #endif
>  
>  struct unix_address {
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 2c0b1c02920f..593ac4b0105f 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -71,6 +71,7 @@
>  #include <linux/unwind_deferred.h>
>  #include <linux/uaccess.h>
>  #include <linux/pidfs.h>
> +#include <net/af_unix.h>
>  
>  #include <uapi/linux/wait.h>
>  
> @@ -1009,6 +1010,12 @@ void __noreturn do_exit(long code)
>  	exit_task_work(tsk);
>  	exit_thread(tsk);
>  
> +	/*
> +	 * Must be after exit_files() and exit_task_work(tsk) to ensure that
> +	 * the task's AF_UNIX sockets have all been closed.
> +	 */
> +	unix_schedule_gc(NULL);

Sashiko noted this can cause relevant regressions:

https://sashiko.dev/#/patchset/cover.1784712370.git.namcao%40linutronix.de

Generally speaking hooking the unix GC at task exit time does not feel
right (layering violation). Perhaps scheduling the GC with a reasonable
timeout in unix_update_graph() would be better?

/P