On Wed, Jan 02, 2019 at 08:56:04AM -0700, Jason Gunthorpe wrote:
> On Tue, Dec 25, 2018 at 06:23:50AM +0200, Leon Romanovsky wrote:
> > 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().
>
> ?? I don't see that.
>
> struct ib_umad_file {
> struct mutex mutex;
> struct ib_umad_port *port;
> struct list_head recv_list;
> struct list_head send_list;
> struct list_head port_list;
> spinlock_t send_lock;
> wait_queue_head_t recv_wait;
> struct ib_mad_agent *agent[IB_UMAD_MAX_AGENTS];
> int agents_dead;
> u8 use_pkey_index;
> u8 already_used;
> };
>
> No 'ib_umad_device' in there.
At the beginning of ib_umad_close(), there is a line:
struct ib_umad_device *dev = file->port->umad_dev;
Because "dev" is not used before ib_umad_dev_put(), compiler probably
defers evaluation of dev pointer till the end, after "kfree(file);".
I don't know what about others, but for us this patch gave two weeks
of clean runs without traces, which is not the case without it.
Thanks
>
> Jason
Attachment:
signature.asc
Description: PGP signature
