Re: [PATCH v5 net 1/2] af_unix: Fix garbage collection of embryos carrying OOB/SCM_RIGHTS.

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Michal Luczaj <mhal@rbox.co>
Date: Wed, 15 May 2024 11:34:51 +0200
> On 5/15/24 02:32, Kuniyuki Iwashima wrote:
> > ...
> > The python script below [0] sends a listener's fd to its embryo as OOB
> > data.  Then, GC does not iterates the embryo from the listener to drop
> > the OOB skb's refcount, and the skb in embryo's receive queue keeps the
> > listener's refcount.  As a result, the listener is leaked and the warning
> > [1] is hit.
> > ...
> 
> Sorry, this does not convey what I wrote. And I think your edit is
> incorrect.
> 
> GC starts from the in-flight listener and *does* iterate the embryo; see
> scan_children() where scan_inflight() is called for all the embryos.

I meant the current code does not call skb_unref() for embryos's OOB skb
because it's done _after_ scan_inflight(), not in scan_inflight().


> The skb in embryo's RQ *does not* keep the listener's refcount; skb from RQ
> ends up in the hit list and is purged.

unix_sk(sk)->oob_skb is a pointer to skb in recvq.  Perhaps I should
have written "the skb which was in embryo's receive queue stays as
unix_sk(sk)->oob_skb and keeps the listener's refcount".


> It is embryo's oob_skb that holds the refcount; see how __unix_gc() goes
> over gc_candidates attempting to kfree_skb(u->oob_skb), notice that `u`
> here is a listener, not an embryo.
> 
> I understand you're "in rush for the merge window", but would it be okay if
> I ask you not to edit my commit messages so heavily?

I noticed the new gc code was merged in Linus' tree.  It's still not
synced with net.git, but I guess it will be done soon and your patch
will not apply on net.git.  Then, I cannot include your patch as a
series, so please feel free to send it to each stable tree.

Thanks