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
