Re: [PATCH rdma-rc v1] IB/core: Fix potential NULL pointer dereference in pkey cache

[Leon Romanovsky <leon@kernel.org>]

On Wed, May 06, 2020 at 03:57:37PM -0300, Jason Gunthorpe wrote:
> On Wed, May 06, 2020 at 09:41:23PM +0300, jackm wrote:
> > On Wed, 6 May 2020 15:09:36 -0300
> > Jason Gunthorpe <jgg@ziepe.ca> wrote:
> >
> > > > > > +out:
> > > > > > +	ib_cache_release_one(device);
> > > > > > +	return err;
> > > > >
> > > > > ib_cache_release_once can be called only once, and it is always
> > > > > called by ib_device_release(), it should not be called here
> > > >
> > > > It doesn't sound right if we rely on ib_device_release() to unwind
> > > > error in ib_cache_setup_one(). I don't think that we need to return
> > > > from ib_cache_setup_one() without cleaning it.
> > >
> > > We do as ib_cache_release_one() cannot be called multiple times
> > >
> > > The general design of all this pre-registration stuff is that the
> > > release function does the clean up and the individual functions should
> > > not error unwind cleanup done in the unconditional release.
> > >
> > > Other schemes were too complicated
> > >
> > > Jason
> >
> > What about calling gid_table_release_one(device) instead of
> > ib_cache_release_one(device) in the error flow ?
>
> Why?

Because it doesn't look clean.

>
> That is not the design, everything that is freed by release is defered
> to release, even on error paths.

I'll resend now.

Thanks

>
> Jason