netdev - Re: [PATCH v5 net 1/2] af_unix: Fix garbage collection of embryos carrying OOB/SCM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c6eb5987-4ffa-47cf-a0c7-dcc7b969d2ca@rbox.co>
Date: Wed, 15 May 2024 11:34:51 +0200
From: Michal Luczaj <mhal@...x.co>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v5 net 1/2] af_unix: Fix garbage collection of embryos
 carrying OOB/SCM_RIGHTS.

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.
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.
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?

Thanks,
Michal