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

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Shoaib Rao <rao.shoaib@oracle.com>
Date: Tue, 10 Sep 2024 16:42:33 -0700
> On 9/10/2024 3:59 PM, Kuniyuki Iwashima wrote:
> > From: Shoaib Rao <rao.shoaib@oracle.com>
> > 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@oracle.com>
> >>> 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.

It does matter.  Please read carefully again...


> > 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.

The returned value from recv_actor() was exact 1 when KASAN was off.
It was -EFAULT only when KASAN was on.

Anyway, -EFAULT is not that important and I'm not so interested in how
KASAN triggers that.  What's important is the fact that the first UAF
is UNIXCB() and the bug happens before that.

[...]
> > Note this is on top of net-next where no additional refcnt is taken
> > for OOB

Also in my patch:

  The recent commit 8594d9b85c07 ("af_unix: Don't call skb_get() for OOB
  skb.") uncovered the issue.