Re: [PATCH net-next v3 3/3] af_unix: Clean up unix_schedule_gc()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] An unconditional call to `unix_schedule_gc()` in `do_exit()` causes a severe system-wide GC storm and CPU exhaustion if any live cyclic AF_UNIX sockets exist.
--

--- Patch [3]: [PATCH net-next v3 3/3] af_unix: Clean up unix_schedule_gc() ---
commit fc308672f15e1ad0994af5c62a43b996e97d2e7a
Author: Nam Cao <namcao@xxxxxxxxxxxxx>

af_unix: Clean up unix_schedule_gc()

This commit simplifies unix_schedule_gc() by removing its user argument,
since it is currently only called with NULL. The conditionals checking the
user parameter within the function body are also removed.

> diff --git a/kernel/exit.c b/kernel/exit.c
> index 593ac4b0105f2..50c5eff07bd65 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1014,7 +1014,7 @@ void __noreturn do_exit(long code)
>  	 * 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);
> +	unix_schedule_gc();
>  
>  	sched_autogroup_exit_task(tsk);
>  	cgroup_task_exit(tsk);

[ ... ]

> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index f180c59b3da9d..d46aeb9d20518 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -635,23 +635,11 @@ static void unix_gc(struct work_struct *work)
>  
>  static DECLARE_WORK(unix_gc_work, unix_gc);
>  
> -#define UNIX_INFLIGHT_SANE_USER		(SCM_MAX_FD * 8)
> -
> -void unix_schedule_gc(struct user_struct *user)
> +void unix_schedule_gc(void)
>  {
>  	if (READ_ONCE(unix_graph_state) == UNIX_GRAPH_NOT_CYCLIC)
>  		return;
>  
> -	/* Penalise users who want to send AF_UNIX sockets
> -	 * but whose sockets have not been received yet.
> -	 */
> -	if (user &&
> -	    READ_ONCE(user->unix_inflight) < UNIX_INFLIGHT_SANE_USER)
> -		return;
> -
>  	if (!READ_ONCE(gc_in_progress))
>  		queue_work(system_dfl_wq, &unix_gc_work);

[Severity: High]
Does this unconditional call to unix_schedule_gc() from do_exit() create a
risk of system-wide GC storms?

If an unprivileged local user creates a cyclic graph of AF_UNIX sockets (for
example, by sending the FDs of a socketpair to each other) and keeps the file
descriptors open, it forms a "live" cycle.

In this scenario:
1. unix_graph_state becomes UNIX_GRAPH_CYCLIC (as set in unix_walk_scc_fast).
2. When unix_gc_work runs, it ignores the live cycle but leaves
   unix_graph_state as UNIX_GRAPH_CYCLIC.
3. Once GC finishes, gc_in_progress becomes false.

Since do_exit() unconditionally calls unix_schedule_gc(), any unrelated task
exiting anywhere in the system will immediately re-queue the GC.

Could this result in CPU exhaustion and a denial of service due to heavy lock
contention on unix_gc_lock, as the GC continuously walks the inflight socket
graph?

> -
> -	if (user && READ_ONCE(unix_graph_cyclic_sccs))
> -		flush_work(&unix_gc_work);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784712370.git.namcao@xxxxxxxxxxxxx?part=3