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

[Kuniyuki Iwashima]

On Tue, Jul 28, 2026 at 6:21 PM Muhammad Bilal <meatuni001@xxxxxxxxx> wrote:
>
> 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:

How can close() be called for struct file while recv() is still being
called for it ?

Did you trigger the real UAF or is this AI hallucination ?

If former, please provide the repro and the KASAN splat.


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