Re: [PATCH 1/2] af_unix: Do not wait for garbage collector in sendmsg()

[Nam Cao <namcao@linutronix.de>]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:
> On 2026-07-04 08:03:56 [+0200], Nam Cao wrote:
>> Kuniyuki Iwashima <kuniyu@google.com> writes:
>> > your patch makes it much easier to abuse.
>> > UNIX_INFLIGHT_SANE_USER is usually much smaller than
>> > RLIMIT_NOFILE.
>> >
>> > unix_schedule_gc() in sendmsg() is to self-regulate malicious users,
>> > otherwise GC relies on unrelated AF_UNIX socket's close() and could
>> > be triggered too late since GC is system-wide.
>> 
>> About the abuse, the scenario where inflight sockets bypass
>> UNIX_INFLIGHT_SANE_USER and delay GC until an unrelated AF_UNIX socket
>> closes actually exists today.
>
> We don't bypass the limit for an ordinary user. That one gets blocked in
> too_many_unix_fds(). But for the CAP_SYS_RESOURCE + CAP_SYS_ADMIN it is
> a different story.
> In that case we cross the UNIX_INFLIGHT_SANE_USER and schedule the GC,
> it is not delayed. The worker is triggered regularity and the thread
> waits for its completion it is just the GC worker does not clean up
> anything so it continues to increase. The only thing that actually
> limits the sender is if it runs out of socket memory and needs to wait.

That's also my understanding. But tasks that have
CAP_SYS_RESOURCE|CAP_SYS_ADMIN should be trustworthy to not burn
system's resources.

> So it kind of works for a single thread.
>
> …
>> To address this properly, we can schedule the GC at task exit. I can
>> include that patch in my series, if that sounds good to you.
>
> task exit or closing the socket?

I meant task exit, but closing the socket also makes sense because that
is when dead cyclic reference can appear. However, unless the file
descriptor's refcount reaches 0, the fs layer will not call us. We could
change fs/, but I am not sure if we should invade into fs/ just for this
one.

For the "resource is still consumed indefinitely after task exit" issue,
then scheduling GC at task exit is good enough.

If the task does not exit, those dead cyclic references can sit there
indefinitely. But this is not really critical as it is capped by
RLIMIT_NOFILE. Additionally it is impossible to prevent a "malicious"
task from creating RLIMIT_NOFILE inflight sockets and just leave them
there.

> Either way, in your PoC it hardly makes any sense to schedule the worker
> over and over and wait for it since it does not do anything. Sure it is
> the system as a whole so triggering might make sense but I'm not sure
> about waiting for its completion.

Agree. For the GC triggering thingy, it is a trade-off between CPU
consumption and how early garbage gets cleared, so that can arguably
stay. But the waiting part has to go.

> Side note: gc_in_progress is redundant since you can't schedule a worker
> twice and work_pending(&unix_gc_work) would provide the same
> information. The only difference is that you avoid scheduling the worker
> while it is running but I don't think this is a problem.

Yep. Additionally gc_in_progress is not read/write under any lock, so
it is possible to read a stale gc_in_progress and the GC gets delayed
indefinitely. That is already on my todo list, among a few other
things. But let's get this most important piece done first.

Nam