Re: [syzbot] [net?] KASAN: slab-use-after-free Read in unix_stream_read_actor (2)
From: Shoaib Rao
Date: Tue Sep 10 2024 - 19:43:05 EST
On 9/10/2024 3:59 PM, Kuniyuki Iwashima wrote:
From: Shoaib Rao <rao.shoaib@xxxxxxxxxx>
Date: Tue, 10 Sep 2024 15:30:08 -0700
My fellow engineer let's first take a breath and calm down. We both are
trying to do the right thing. Now read my comments below and if I still
don't get it, please be patient, maybe I am not as smart as you are.
On 9/10/2024 2:53 PM, Kuniyuki Iwashima wrote:
From: Shoaib Rao <rao.shoaib@xxxxxxxxxx>
Date: Tue, 10 Sep 2024 13:57:04 -0700
The commit Message:
syzbot reported use-after-free in unix_stream_recv_urg(). [0]
The scenario is
1. send(MSG_OOB)
2. recv(MSG_OOB)
-> The consumed OOB remains in recv queue
3. send(MSG_OOB)
4. recv()
-> manage_oob() returns the next skb of the consumed OOB
-> This is also OOB, but unix_sk(sk)->oob_skb is not cleared
5. recv(MSG_OOB)
-> unix_sk(sk)->oob_skb is used but already freed
How did you miss this ?
Again, please read my patch and mails **carefully**.
unix_sk(sk)->oob_sk wasn't cleared properly and illegal access happens
in unix_stream_recv_urg(), where ->oob_skb is dereferenced.
Here's _technical_ thing that you want.
This is exactly what I am trying to point out to you.
The skb has proper references and is NOT de-referenced because
__skb_datagram_iter() detects that the length is zero and returns EFAULT.
It's dereferenced as UNIXCB(skb).consumed first in
unix_stream_read_actor().
That does not matter as the skb still has a refernce. That is why I
asked you to print the reference count.
Then, 1 byte of data is copied without -EFAULT because
unix_stream_recv_urg() always passes 1 as chunk (size) to
recv_actor().
Can you verify this because IIRC it is not de-refernced. AFAIK, KASAN
does nothing that would cause returning EFAULT and if KASAN does spot
this illegal access why is it not pancing the system or producing a report.
This is where we disagree.
Shoaib
That's why I said KASAN should be working on your setup and suggested
running the repro with/without KASAN. If KASAN is turned off, single
byte garbage is copied from the freed area.
See the last returned values below
Without KASAN:
---8<---
write(1, "executing program\n", 18
executing program
) = 18
socketpair(AF_UNIX, SOCK_STREAM, 0, [3, 4]) = 0
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\333", iov_len=1}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_OOB|MSG_DONTWAIT
[ 15.657345] queued OOB: ff1100000442c700
) = 1
recvmsg(3,
[ 15.657793] reading OOB: ff1100000442c700
{msg_name=NULL, msg_namelen=0, msg_iov=NULL, msg_iovlen=0, msg_controllen=0, msg_flags=MSG_OOB}, MSG_OOB|MSG_WAITFORONE) = 1
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\21", iov_len=1}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_OOB|MSG_NOSIGNAL|MSG_MORE
[ 15.659830] queued OOB: ff1100000442c300
) = 1
recvfrom(3,
[ 15.660272] free skb: ff1100000442c300
"\21", 125, MSG_DONTROUTE|MSG_TRUNC, NULL, NULL) = 1
recvmsg(3,
[ 15.661014] reading OOB: ff1100000442c300
{msg_namelen=0, MSG_OOB|MSG_ERRQUEUE) = 1
---8<---
With KASAN:
---8<---
socketpair(AF_UNIX, SOCK_STREAM, 0, [3, 4]) = 0
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\333", iov_len=1}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_OOB|MSG_DONTWAIT
[ 134.735962] queued OOB: ff110000099f0b40
) = 1
recvmsg(3,
[ 134.736460] reading OOB: ff110000099f0b40
{msg_name=NULL, msg_namelen=0, msg_iov=NULL, msg_iovlen=0, msg_controllen=0, msg_flags=MSG_OOB}, MSG_OOB|MSG_WAITFORONE) = 1
sendmsg(4, {msg_name=NULL, msg_namelen=0, msg_iov=[{iov_base="\21", iov_len=1}], msg_iovlen=1, msg_controllen=0, msg_flags=0}, MSG_OOB|MSG_NOSIGNAL|MSG_MORE
[ 134.738554] queued OOB: ff110000099f0c80
) = 1
recvfrom(3,
[ 134.739086] free skb: ff110000099f0c80
"\21", 125, MSG_DONTROUTE|MSG_TRUNC, NULL, NULL) = 1
recvmsg(3,
[ 134.739792] reading OOB: ff110000099f0c80
{msg_namelen=0}, MSG_OOB|MSG_ERRQUEUE) = -1 EFAULT (Bad address)
---8<---
See more below
---8<---
# ./oob
executing program
[ 25.773750] queued OOB: ff1100000947ba40
[ 25.774110] reading OOB: ff1100000947ba40
[ 25.774401] queued OOB: ff1100000947bb80
[ 25.774669] free skb: ff1100000947bb80
[ 25.774919] reading OOB: ff1100000947bb80
[ 25.775172] ==================================================================
[ 25.775654] BUG: KASAN: slab-use-after-free in unix_stream_read_actor+0x86/0xb0
---8<---
---8<---
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index a1894019ebd5..ccd9c47160a5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2230,6 +2230,7 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
__skb_queue_tail(&other->sk_receive_queue, skb);
spin_unlock(&other->sk_receive_queue.lock);
+ printk(KERN_ERR "queued OOB: %px\n", skb);
sk_send_sigurg(other);
unix_state_unlock(other);
other->sk_data_ready(other);
@@ -2637,6 +2638,7 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state)
spin_unlock(&sk->sk_receive_queue.lock);
unix_state_unlock(sk);
+ printk(KERN_ERR "reading OOB: %px\n", oob_skb);
chunk = state->recv_actor(oob_skb, 0, chunk, state);
if (!(state->flags & MSG_PEEK))
@@ -2915,7 +2917,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
skb_unlink(skb, &sk->sk_receive_queue);
consume_skb(skb);
-
+ printk(KERN_ERR "free skb: %px\n", skb);
This printk is wrongly placed
It's not, because this just prints the address of OOB skb just after
it's illegally consumed without MSG_OOB. The code is only called
for the illegal OOB during the repro.
because the skb has been freed above, but
since it is just printing the pointer it should be OK, access to any skb
field will be an issue. You should move this printk to before
unix_stream_read_generic and print the refcnt on the skb and the length
of the data and verify what I am saying, that the skb has one refcnt and
zero length.
Note this is on top of net-next where no additional refcnt is taken
for OOB, so no need to print skb's refcnt. Also the length is not
related because chunk is always 1.
This is the kind of interaction I was looking for. If I have missed
something please be patient and let me know.
Regards,
Shoaib