Re: [PATCH net v2] af_unix: Fix UAF read of tail->len in unix_stream_data_wait()

[Kuniyuki Iwashima]

On Mon, May 18, 2026 at 9:51 AM Jann Horn <jannh@xxxxxxxxxx> wrote:
>
> unix_stream_data_wait() does skb_peek_tail(&sk->sk_receive_queue) without
> holding any lock that prevents SKBs on that queue from being dequeued and
> freed.
> This has been the case since commit 79f632c71bea ("unix/stream: fix
> peeking with an offset larger than data in queue").
> The first consequence of this is that the pointer comparison
> `tail != last` can be false even if `last` semantically refers to an
> already-freed SKB while `tail` is a new SKB allocated at the same address;
> which can cause unix_stream_data_wait() to wrongly keep blocking after new
> data has arrived, but only in a weird scenario where a peeking recv() and
> a normal recv() on the same socket are racing, which is probably not a
> real problem.
>
> But since commit 2b514574f7e8 ("net: af_unix: implement splice for stream
> af_unix sockets"), `tail` is actually dereferenced, which can cause UAF in
> the following race scenario (where test_setup() runs single-threaded,
> and afterwards, test_thread1() and test_thread2() run concurrently in
> two threads:
> ```
> static int socks[2];
> void test_setup(void) {
>   socketpair(AF_UNIX, SOCK_STREAM, 0, socks);
>   send(socks[1], "A", 1, 0);
>   int peekoff = 1;
>   setsockopt(socks[0], SOL_SOCKET, SO_PEEK_OFF, &peekoff, sizeof(peekoff));
> }
> void test_thread1(void) {
>   char dummy;
>   recv(socks[0], &dummy, 1, MSG_PEEK);
> }
> void test_thread2(void) {
>   char dummy;
>   recv(socks[0], &dummy, 1, 0);
>   shutdown(socks[1], SHUT_WR);
> }
> ```
>
> when racing like this:
> ```
> thread1                       thread2
> unix_stream_read_generic
>   mutex_lock(&u->iolock)
>   skb_peek(&sk->sk_receive_queue)
>   skb_peek_next(skb, &sk->sk_receive_queue)
>   mutex_unlock(&u->iolock)
>                               unix_stream_read_generic
>                                 unix_state_lock(sk)
>                                 skb_peek(&sk->sk_receive_queue)
>                                 unix_state_unlock(sk)
>   unix_stream_data_wait
>     unix_state_lock(sk)
>     tail = skb_peek_tail(&sk->sk_receive_queue)
>                                 spin_lock(&sk->sk_receive_queue.lock)
>                                 __skb_unlink(skb, &sk->sk_receive_queue)
>                                 spin_unlock(&sk->sk_receive_queue.lock)
>                                 consume_skb(skb) [frees the SKB]
>     `tail != last`: false
>     `tail`: true
>     `tail->len != last_len` ***UAF***
> ```
>
> Fix the UAF by removing the read of tail->len; checking tail->len would
> only make sense if SKBs in the receive queue of a UNIX socket could grow,
> which can no longer happen.
>
> Kuniyuki explained:
>
> > When commit 869e7c62486e ("net: af_unix: implement stream sendpage
> > support") added sendpage() support, data could be appended to the last
> > skb in the receiver's queue.
> >
> > That's why we needed to check if the length of the last skb was changed
> > while waiting for new data in unix_stream_data_wait().
> >
> > However, commit a0dbf5f818f9 ("af_unix: Support MSG_SPLICE_PAGES") and
> > commit 57d44a354a43 ("unix: Convert unix_stream_sendpage() to use
> > MSG_SPLICE_PAGES") refactored sendmsg(), and now data is always added
> > to a new skb.
>
> That means this fix is not suitable for kernels before 6.5.
>
> Fixes: 2b514574f7e8 ("net: af_unix: implement splice for stream af_unix sockets")
> Cc: stable@xxxxxxxxxxxxxxx # 6.5.x
> Signed-off-by: Jann Horn <jannh@xxxxxxxxxx>

Reviewed-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>

Thanks !