Re: [PATCH net-next v3 2/3] af_unix: Do not wait for garbage collector in sendmsg()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Removing `unix_schedule_gc()` from `sendmsg()` prevents the AF_UNIX garbage collector from being proactively triggered for isolated cyclic references, leading to spurious -ETOOMANYREFS failures and indefinite resource leaks.
- [Low] The `user` parameter and its associated conditional logic in `unix_schedule_gc()` are now dead code.
--

--- Patch [2]: [PATCH net-next v3 2/3] af_unix: Do not wait for garbage collector in sendmsg() ---
commit e95749602b1bd36f1394eab73582d88874294091
Author: Nam Cao <namcao@xxxxxxxxxxxxx>
af_unix: Do not wait for garbage collector in sendmsg()

This commit removes the unix_schedule_gc() call from the af_unix sendmsg()
path to prevent priority inversion scenarios. The change relies on the
assumption that checking RLIMIT_NOFILE and running the garbage collector
asynchronously are sufficient to prevent historical soft lockup and
out-of-memory issues.

> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index 0783555e25266..f180c59b3da9d 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -300,8 +300,6 @@ int unix_prepare_fpl(struct scm_fp_list *fpl)
>  	if (!fpl->edges)
>  		goto err;
>  
> -	unix_schedule_gc(fpl->user);
> -

[Severity: High]
Does removing this call prevent the af_unix garbage collector from being
proactively triggered for isolated cyclic references?

If a long-running task generates cyclic af_unix references and drops its
file descriptors, the cyclic references' f_count remains > 0, so
unix_release_sock() is never called. Without this trigger, wouldn't the
garbage collector remain unscheduled unless the task exits or closes an
unrelated, non-cyclic af_unix socket?

Could this regression lead to legitimate operations failing with
-ETOOMANYREFS for unprivileged processes, or allow a privileged process
to bypass RLIMIT_NOFILE and leak memory unboundedly?

[Severity: Low]
Does removing this call leave the user parameter in unix_schedule_gc()
as dead code?

If all other callers now unconditionally pass NULL, the conditional logic
checking user in unix_schedule_gc() will permanently evaluate to false:

net/unix/garbage.c:unix_schedule_gc() {
    ...
	if (user &&
	    READ_ONCE(user->unix_inflight) < UNIX_INFLIGHT_SANE_USER)
		return;
    ...
	if (user && READ_ONCE(unix_graph_cyclic_sccs))
		flush_work(&unix_gc_work);
    ...
}

Should the user parameter and these checks be removed as well?

>  	return 0;
>  
>  err:

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