Re: [PATCH v3] drivers/base/core: refcount kobject and bus on device attribute read / store

[Luis Chamberlain <mcgrof@kernel.org>]

On Wed, Jun 23, 2021 at 06:51:42PM +0200, Greg KH wrote:
> On Wed, Jun 23, 2021 at 09:14:34AM -0700, Luis Chamberlain wrote:
> > sysfs isn't doing any active reference check for the kobject device
> > attribute as it doesn't care for them. So syfs calls
> > dev_attr_store(), but the dev_attr_store() is not preventing the device
> > attribute ops to go fishing, and we destroy them while we're destroying
> > the device on module removal.
> 
> Ah, but sysfs _should_ be doing this properly.
> 
> I think the issue is that when we store the kobject pointer in kernfs,
> it is NOT incremented.  Look at sysfs_create_dir_ns(), if we call
> kobject_get(kobj) right before we call kernfs_create_dir_ns(), and then
> properly clean things up later on when we remove the sysfs directory
> (i.e. the kobject), it _should_ fix this problem.
> 
> Then, we know, whenever show/store/whatever is called, when we cast out
> of kernfs the private pointer to a kobject, that the kobject really is
> still alive, so we can use it properly.
> 
> Can you try that, it should be a much "simpler" change here.

Agreed, its cleaner. It should also address the type race consideration
I had, given that in the zram case, for instance, we will call device_del() on
del_gendisk() and the order of freeing typically is something like:

	del_gendisk(zram->disk);
	blk_cleanup_queue(zram->disk->queue);
	put_disk(zram->disk);

The put_disk() is what would make our gendisk->private_data invalid.
Will spin up a v4 with your suggestion.

  Luis