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 09:55:03 -0700
> On 9/9/2024 5:48 PM, Kuniyuki Iwashima wrote:
> > From: Shoaib Rao <rao.shoaib@xxxxxxxxxx>
> > Date: Mon, 9 Sep 2024 17:29:04 -0700
> >> I have some more time investigating the issue. The sequence of packet
> >> arrival and consumption definitely points to an issue with OOB handling
> >> and I will be submitting a patch for that.
> > 
> > It seems a bit late.
> > My patches were applied few minutes before this mail was sent.
> > https://urldefense.com/v3/__https://lore.kernel.org/netdev/172592764315.3964840.16480083161244716649.git-patchwork-notify@xxxxxxxxxx/__;!!ACWV5N9M2RV99hQ!M806VrqNEGFgGXEoWG85msKAdFPXup7RzHy9Kt4q_HOfpPWsjNHn75KyFK3a3jWvOb9EEQuFGOjpqgk$
> > 
> 
> That is a subpar fix. I am not sure why the maintainers accepted the fix 
> when it was clear that I was still looking into the issue.

Just because it's not a subpar fix and you were slow and wrong,
clining to triggering the KASAN splat without thinking much.


> Plus the 
> claim that it fixes the panic is absolutely wrong.

The _root_ cause of the splat is mishandling of OOB in manage_oob()
which causes UAF later in another recvmsg().

Honestly your patch is rather a subpar fix to me, few points:

  1. The change conflicts with net-next as we have already removed
     the additional unnecessary refcnt for OOB skb that has caused
     so many issue reported by syzkaller

  2. Removing OOB skb in queue_oob() relies on the unneeded refcnt
     but it's not mentioned; if merge was done wrongly, another UAF
     will be introduced in recvmsg()

  3. Even the removing logic is completely unnecessary if manage_oob()
     is changed

  4. The scan_again: label is misplaced; two consecutive empty OOB skbs
     never exist at the head of recvq

  5. ioctl() is not fixed

  6. No test added

  7. Fixes: tag is bogus

  8. Subject lacks target tree and af_unix prefix