Re: [PATCH 5.15.y] af_unix: Clear oob_skb in scan_inflight()

[Tamir Awad <tamir@auth.ink>]

[-- Attachment #1: Type: text/plain, Size: 37 bytes --]

I included the patch as a file below.

[-- Attachment #2: af_unix-5.15.y-patch.txt --]
[-- Type: text/plain, Size: 2781 bytes --]

From 58f7f0cf9495d4b74cff8015bd609a7e594718c5 Mon Sep 17 00:00:00 2001
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Mon, 3 Mar 2025 19:01:49 -0800
Subject: [PATCH 5.15.y] af_unix: Clear oob_skb in scan_inflight()

unix_gc() accesses oob_skb after dropping sk_receive_queue.lock. This
can race with queue_oob(). Clear oob_skb in scan_inflight() while the
lock is held.

This resends Kuniyuki Iwashima's March 2025 fix for the old GC. The
code diff is unchanged.

On 5.15.223, the reproducer panicked three unpatched QEMU guests. Three
patched guests completed 8,000 rounds each without a kernel warning.
Both builds used the same config and guest image. An instrumented run
traced the harness's failed racing sends to EPIPE and ECONNRESET and
confirmed all 8,000 iterations reached the race.

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Link: https://lore.kernel.org/all/20250304030149.82265-1-kuniyu@amazon.com/
Reported-by: Lei Lu <llfamsec@gmail.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Tamir Awad <security@auth.ink>
---
 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 d2fc795..a733d0d 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.
 	 */
-- 
2.56.0.windows.2