Re: [PATCH rdma-next v1] IB/umad: Fix use-after-free in cdev_put

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 24, 2018 at 02:58:46PM -0700, Jason Gunthorpe wrote:
> On Mon, Dec 24, 2018 at 03:46:33PM +0200, Leon Romanovsky wrote:
>
> > diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
> > index 1a90012b5542..6929796e35e6 100644
> > +++ b/drivers/infiniband/core/user_mad.c
> > @@ -1031,8 +1031,8 @@ static int ib_umad_close(struct inode *inode, struct file *filp)
> >
> >  	mutex_unlock(&file->port->file_mutex);
> >
> > -	kfree(file);
> >  	ib_umad_dev_put(dev);
> > +	kfree(file);
> >  	return 0;
>
> This doesn't make any sense. The kasn splat is suggesting that 'dev'
> is the thing that was freed, not file, and ib_umad_dev_put does not
> touch 'file' memory either, so I don't see how order here matters..

dev is part 0f "file" structure which is accessed in ib_umad_dev_put().

>
> .. and as I said earlier, the put is redundant, so this will not
> release dev anyhow..
>
> Something else is going wrong to get that trace..
>
> It sure looks like the cdev_put during close() is faulting, which
> means the 'dev' was freed by the timem close returns, which should not
> be possible. That points to a problem with the cdev reference chaining
> and some kind if racing of ib_umad_kill_port ?

Parav will remove "puts" after he will return from US vacation,
but for now this patch fixed for us regression and we don't have
any KASAN warnings in that area anymore.

Thanks

>
> Jason

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux