Re: [PATCH 1/2] block: grab a device refcount in disk_uevent

[Ming Lei <ming.lei@redhat.com>]

On Thu, Jul 01, 2021 at 11:02:32AM +0200, Christoph Hellwig wrote:
> On Thu, Jul 01, 2021 at 04:54:05PM +0800, Ming Lei wrote:
> > On Thu, Jul 01, 2021 at 10:16:37AM +0200, Christoph Hellwig wrote:
> > > Sending uevents requires the struct device to be alive.  To
> > > ensure that grab the device refcount instead of just an inode
> > > reference.
> > > 
> > > Fixes: bc359d03c7ec ("block: add a disk_uevent helper")
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > > ---
> > >  block/genhd.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/block/genhd.c b/block/genhd.c
> > > index 79aa40b4c39c..af4d2ab4a633 100644
> > > --- a/block/genhd.c
> > > +++ b/block/genhd.c
> > > @@ -365,12 +365,12 @@ void disk_uevent(struct gendisk *disk, enum kobject_action action)
> > >  	xa_for_each(&disk->part_tbl, idx, part) {
> > >  		if (bdev_is_partition(part) && !bdev_nr_sectors(part))
> > >  			continue;
> > > -		if (!bdgrab(part))
> > > +		if (!kobject_get_unless_zero(&part->bd_device.kobj))
> > >  			continue;
> > 
> > ->bd_device is embedded in the block device, and it has same lifetime
> > with the block device, even part_release() calls bdput() to release this
> > device, so why doesn't work by holding a inode reference?
> 
> Because sending a uevent on a device that has device_del called on it
> is going to blow up.

But grabbing one reference can't prevent device_del() from being called.

IMO, if driver core doesn't allow to sending uevent on one deleted device,
it should return a failure instead of kernel panic.

Thanks,
Ming