| Description | In the Linux kernel, the following vulnerability has been resolved:
af_unix: Fix UAF read of tail->len in unix_stream_data_wait()
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. |
|---|
