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

[Muhammad Bilal]

On Wed, Jul 30, 2026 Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote:
> 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.

You're right, no splat. I didn't verify the interleaving was
actually reachable before sending this.

recv_urg(), manage_oob(), and read_skb() all take u->iolock, so they
can't race each other. The only unlocked writer is
unix_release_sock(), which only runs on the file's final release,
and a thread inside recvmsg() holds an active reference via
fdget()/fdput() for the whole syscall, so that can't happen
concurrently with a recv in flight on the same file.

Also ran an oversubscribed stress test against this exact scenario
on 7.2.0-rc5+ with generic KASAN enabled, closing the real fd against
a thread blocked in recv(MSG_OOB). No hit across two runs.

Withdrawing this patch. Sorry for the noise.

Muhammad Bilal

On Thu, Jul 30, 2026 at 11:34 PM Kuniyuki Iwashima <kuniyu@xxxxxxxxxx> wrote:
>
> 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