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

[Muhammad Bilal]

unix_stream_recv_urg() saves a bare pointer to the OOB skb and then
drops sk_receive_queue.lock and unix_state_lock before accessing it
at lines that call recv_actor() and update UNIXCB().consumed.

After commit 8594d9b85c07 ("af_unix: Don't call skb_get() for OOB
skb.") removed the extra reference count, oob_skb is just a pointer
into the receive queue with no additional refcount protection.

A concurrent close() via unix_release_sock() does not acquire u->iolock
and can race as follows:

  1. recv_urg() saves oob_skb pointer under queue lock
  2. recv_urg() drops queue lock and state lock
  3. unix_release_sock() sets u->oob_skb = NULL (locklessly)
  4. unix_release_sock() drains receive queue, kfree_skb() frees oob_skb
  5. recv_urg() accesses freed oob_skb via recv_actor() -> UAF

Additionally, unix_release_sock() cleared u->oob_skb with a plain store
outside any lock, which is a data race against READ_ONCE(u->oob_skb) in
manage_oob() and unix_stream_poll() which expect all writers to use
WRITE_ONCE() under sk_receive_queue.lock.

Fix the UAF by taking skb_get() on oob_skb while holding the queue lock
in unix_stream_recv_urg(), ensuring the skb survives until consume_skb()
drops the extra reference after use.

Fix the data race by clearing u->oob_skb in unix_release_sock() under
sk_receive_queue.lock using WRITE_ONCE(), consistent with all other
writers.

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