Re: [PATCH] scsi: fix race condition when removing target

["Ewan D. Milne" <emilne@redhat.com>]

On Wed, 2017-11-29 at 17:39 +0000, gregkh@linuxfoundation.org wrote:
> On Wed, Nov 29, 2017 at 05:20:50PM +0100, hch@lst.de wrote:
> > On Wed, Nov 29, 2017 at 04:18:30PM +0000, Bart Van Assche wrote:
> > > As the above patch description shows it can happen that the SCSI core calls
> > > get_device() after the device reference count has reached zero and before
> > > the memory for struct device is freed. Although the above patch looks fine
> > > to me, would you consider it acceptable to modify get_device() such that it
> > > uses kobject_get_unless_zero() instead of kobject_get()? I'm asking this
> > > because that change would help to reduce the complexity of the already too
> > > complicated SCSI core.
> > 
> > I don't think we can just modify get_device, but we can add a new
> > get_device_unless_zero.  In fact I have an open coded variant of that
> > in nvme, and was planning to submit one for the current merge window..
> 
> I feel like that is just delaying the real fix, shouldn't there be a bus
> lock somewhere on the put_device path for this bus to prevent this?
> 
> thanks,
> 
> greg k-h

Why is it that clients of the kobject code have to have their own
lock / state checking to prevent a duplicate destructor callback?
It seems to me like this is something the core functionality should
provide, because a get inside a destructor would *always* be wrong, no?

It looks like:

void refcount_inc(refcount_t *r)
{
        WARN_ONCE(!refcount_inc_not_zero(r), "refcount_t: increment on 0; use-after-free.\n");
}

would have warned if CONFIG_REFCOUNT_FULL was on, I/we don't normally
enable that though.

-Ewan