Embryo socket is not queued in gc_candidates, so we can't drop a reference held by its oob_skb.

Let's say we create listener and embryo sockets, send the listener's fd to the embryo as OOB data, and close() them without recv()ing the OOB data.

There is a self-reference cycle like

listener -> embryo.oob_skb -> listener

, so this must be cleaned up by GC. Otherwise, the listener's refcnt is not released and sockets are leaked:

# unshare -n # cat /proc/net/protocols | grep UNIX-STREAM UNIX-STREAM 1024 0 -1 NI 0 yes kernel ...

# python3

...

...

from array import array from socket import *

s = socket(AF_UNIX, SOCK_STREAM) s.bind('\0test\0') s.listen()

c = socket(AF_UNIX, SOCK_STREAM) c.connect(s.getsockname()) c.sendmsg([b'x'], [(SOL_SOCKET, SCM_RIGHTS, array('i', [s.fileno()]))], MSG_OOB)

1

...

...

quit()

# cat /proc/net/protocols | grep UNIX-STREAM UNIX-STREAM 1024 3 -1 NI 0 yes kernel ... ^^^ 3 sockets still in use after FDs are close()d

Let's drop the embryo socket's oob_skb ref in scan_inflight().

This also fixes a racy access to oob_skb that commit 9841991a446c ("af_unix: Update unix_sk(sk)->oob_skb under sk_receive_queue lock.") fixed for the new Tarjan's algo-based GC.

Fixes: 314001f0bf92 ("af_unix: Add OOB support") Reported-by: Lei Lu llfamsec@gmail.com Signed-off-by: Kuniyuki Iwashima kuniyu@amazon.com --- This has no upstream commit because I replaced the entire GC in 6.10 and the new GC does not have this bug, and this fix is only applicable to the old GC (<= 6.9), thus for 5.15/6.1/6.6. --- --- net/unix/garbage.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/unix/garbage.c b/net/unix/garbage.c index 2a758531e102..b3fbdf129944 100644 --- a/net/unix/garbage.c +++ b/net/unix/garbage.c @@ -102,13 +102,14 @@ static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *), /* Process the descriptors of this socket */ int nfd = UNIXCB(skb).fp->count; struct file **fp = UNIXCB(skb).fp->fp; + struct unix_sock *u;

while (nfd--) { /* Get the socket the fd matches if it indeed does so */ struct sock *sk = unix_get_socket(*fp++);

if (sk) { - struct unix_sock *u = unix_sk(sk); + u = unix_sk(sk);

/* Ignore non-candidates, they could * have been added to the queues after @@ -122,6 +123,13 @@ static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *), } } if (hit && hitlist != NULL) { +#if IS_ENABLED(CONFIG_AF_UNIX_OOB) + u = unix_sk(x); + if (u->oob_skb) { + WARN_ON_ONCE(skb_unref(u->oob_skb)); + u->oob_skb = NULL; + } +#endif __skb_unlink(skb, &x->sk_receive_queue); __skb_queue_tail(hitlist, skb); } @@ -299,17 +307,9 @@ void unix_gc(void) * which are creating the cycle(s). */ skb_queue_head_init(&hitlist); - list_for_each_entry(u, &gc_candidates, link) { + list_for_each_entry(u, &gc_candidates, link) scan_children(&u->sk, inc_inflight, &hitlist);

-#if IS_ENABLED(CONFIG_AF_UNIX_OOB) - if (u->oob_skb) { - kfree_skb(u->oob_skb); - u->oob_skb = NULL; - } -#endif - } - /* not_cycle_list contains those sockets which do not make up a * cycle. Restore these to the inflight list. */