Re: [syzbot] [net?] KASAN: slab-use-after-free Read in unix_stream_read_actor (2)

[Kuniyuki Iwashima]

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.

---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);
 			if (scm.fp)
 				break;
 		} else {
---8<---

[...]
> None of this points to where the skb is "dereferenced" The test case 
> added will never panic the kernel.
> 
> In the organizations that I have worked with, just because a change 
> stops a panic does not mean the issue is fixed. You have to explicitly 
> show where and how. That is what i am asking, Yes there is a bug, but it 
> will not cause the panic, so if you are just high and might engiineer, 
> show where and how?
> > 
> > This will be the last mail from me in this thread.  I don't want to
> > waste time on someone who doesn't read mails.
> Yes please don't if you do not have anything technical to say, all your 
> comments are "smart comments". This email thread would end if you could 
> just say, here is line XXXX where the skb is de referenced, but you have 
> not because you have no idea.
> 
> BTTW Your comment about holding the extra refcnt without any reason just 
> shows that you DO NOT understand the code. And the confusion to refcnts 
> has been caused by your changes, I am concerned your changes have broken 
> the code.
> 
> I am willing to accept that I may be wrong but only if I am shown the 
> location of de-reference. Please do not respond if you can not point to 
> the exact location.

Please do not respond if you just ask without thinking.