[PATCH v1] af_unix: fix use-after-free in unix_stream_recv_urg()

[Muhammad Bilal]

unix_stream_recv_urg() reads out-of-band (MSG_OOB) data from
unix_sk(sk)->oob_skb.
It saves a pointer to oob_skb under sk_receive_queue.lock and state_lock,
and then drops both locks before invoking state->recv_actor(oob_skb, ...)
and incrementing UNIXCB(oob_skb).consumed.

Prior to commit 8594d9b85c07 ("af_unix: Don't call skb_get() for OOB skb."),
the OOB sk_buff held an additional reference count. When that extra refcount
was removed, oob_skb became a bare pointer into the socket's receive queue
without any lifetime protection once the receive queue lock is released.

Because unix_release_sock() does not acquire u->iolock during socket close,
a concurrent close() on the socket races with unix_stream_recv_urg() as follows:

  Thread A (recvmsg MSG_OOB)            Thread B (close)
  --------------------------            ----------------
  Acquires iolock & queue lock
  Saves oob_skb = u->oob_skb
  Drops queue lock & state lock
                                        Acquires state_lock, sets TCP_CLOSE
                                        Sets u->oob_skb = NULL (locklessly)
                                        skb_dequeue() & kfree_skb()
frees oob_skb
  recv_actor(oob_skb, ...) -> UAF read
  UNIXCB(oob_skb).consumed++ -> UAF write

Additionally, unix_release_sock() cleared u->oob_skb with a lockless plain
assignment. This creates a KCSAN-visible data race against READ_ONCE(u->oob_skb)
in manage_oob() and unix_stream_poll(), both of which rely on all
writers updating
u->oob_skb with WRITE_ONCE() under sk_receive_queue.lock.

Fix the use-after-free by taking an explicit reference via skb_get(oob_skb)
while holding sk_receive_queue.lock in unix_stream_recv_urg(), and releasing it
with consume_skb(oob_skb) after the data copy and consumed counter
update complete.
This guarantees the sk_buff remains valid even if a concurrent close() dequeues
and drops the queue's reference.

Fix the data race in unix_release_sock() by wrapping the u->oob_skb clear in
sk_receive_queue.lock and using WRITE_ONCE(), matching the locking contract used
by all other oob_skb update paths in af_unix.c.

Found by code review of AF_UNIX OOB lifetime management after commit
8594d9b85c07,
and verified with a clean build.

Fixes: 8594d9b85c07 ("af_unix: Don't call skb_get() for OOB skb.")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Muhammad Bilal <meatuni001@xxxxxxxxx>
---
 net/unix/af_unix.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f7a9d55eee8a..011ea88d57c2 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -689,7 +689,9 @@ static void unix_release_sock(struct sock *sk, int embrion)
 	unix_state_unlock(sk);

 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
-	u->oob_skb = NULL;
+	spin_lock(&sk->sk_receive_queue.lock);
+	WRITE_ONCE(u->oob_skb, NULL);
+	spin_unlock(&sk->sk_receive_queue.lock);
 #endif

 	wake_up_interruptible_all(&u->peer_wait);
@@ -2773,6 +2775,7 @@ static int unix_stream_recv_urg(struct
unix_stream_read_state *state)
 	}

 	oob_skb = u->oob_skb;
+	skb_get(oob_skb);

 	if (!(state->flags & MSG_PEEK)) {
 		WRITE_ONCE(u->oob_skb, NULL);
@@ -2793,6 +2796,8 @@ static int unix_stream_recv_urg(struct
unix_stream_read_state *state)
 	if (!(state->flags & MSG_PEEK))
 		UNIXCB(oob_skb).consumed += 1;

+	consume_skb(oob_skb);
+
 	mutex_unlock(&u->iolock);

 	consume_skb(read_skb);
-- 
2.55.0