Re: KASAN: use-after-free Read in vhost_chr_write_iter

[Jason Wang]

On 2018å05æ22æ 16:38, DaeRyong Jeong wrote:
> On Mon, May 21, 2018 at 10:38:10AM +0800, Jason Wang wrote:
> > On 2018å05æ18æ 17:24, Jason Wang wrote:
> > > On 2018å05æ17æ 21:45, DaeRyong Jeong wrote:
> > > > We report the crash: KASAN: use-after-free Read in vhost_chr_write_iter
> > > >
> > > > This crash has been found in v4.17-rc1 using RaceFuzzer (a modified
> > > > version of Syzkaller), which we describe more at the end of this
> > > > report. Our analysis shows that the race occurs when invoking two
> > > > syscalls concurrently, write$vnet and ioctl$VHOST_RESET_OWNER.
> > > >
> > > >
> > > > Analysis:
> > > > We think the concurrent execution of vhost_process_iotlb_msg() and
> > > > vhost_dev_cleanup() causes the crash.
> > > > Both of functions can run concurrently (please see call sequence below),
> > > > and possibly, there is a race on dev->iotlb.
> > > > If the switch occurs right after vhost_dev_cleanup() frees
> > > > dev->iotlb, vhost_process_iotlb_msg() still sees the non-null value
> > > > and it
> > > > keep executing without returning -EFAULT. Consequently, use-after-free
> > > > occures
> > > >
> > > >
> > > > Thread interleaving:
> > > > CPU0 (vhost_process_iotlb_msg)ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ CPU1 (vhost_dev_cleanup)
> > > > (In the case of both VHOST_IOTLB_UPDATE and
> > > > VHOST_IOTLB_INVALIDATE)
> > > > =====ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ =====
> > > >  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ vhost_umem_clean(dev->iotlb);
> > > > if (!dev->iotlb) {
> > > >  ÂÂÂÂÂÂÂÂÂÂÂ ret = -EFAULT;
> > > >  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ break;
> > > > }
> > > >  ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ dev->iotlb = NULL;
> > > >
> > > >
> > > > Call Sequence:
> > > > CPU0
> > > > =====
> > > > vhost_net_chr_write_iter
> > > >  ÂÂÂÂvhost_chr_write_iter
> > > >  ÂÂÂÂÂÂÂ vhost_process_iotlb_msg
> > > >
> > > > CPU1
> > > > =====
> > > > vhost_net_ioctl
> > > >  ÂÂÂÂvhost_net_reset_owner
> > > >  ÂÂÂÂÂÂÂ vhost_dev_reset_owner
> > > >  ÂÂÂÂÂÂÂÂÂÂÂ vhost_dev_cleanup
> > > Thanks a lot for the analysis.
> > >
> > > This could be addressed by simply protect it with dev mutex.
> > >
> > > Will post a patch.
> > Could you please help to test the attached patch? I've done some smoking
> > test.
> >
> > Thanks
> Sorry to say this, but we don't have a reproducer for this bug since our
> reproducer is being implemented.
>
> This crash had occrued a few times in our fuzzer, so I inspected the code
> manually.
>
> It seems the patch is good for me, but we can't test the patch for now.
> Sorry.

No problem.

I'm trying to craft a reproducer, looks not hard.

Thanks