On Mon, Jun 21, 2021 at 04:36:51PM -0700, Luis Chamberlain wrote: > It's possible today to have a device attribute read or store > race against device removal. When this happens there is a small > chance that the derefence for the private data area of the driver > is NULL. > > Let's consider the zram driver as an example. Its possible to run into > a race where a sysfs knob is being used, we get preempted, and a zram > device is removed before we complete use of the sysfs knob. This can happen > for instance on block devices, where for instance the zram block devices > just part of the private data of the block device. > > For instance this can happen in the following two situations > as examples to illustrate this better: > > CPU 1 CPU 2 > destroy_devices > ... > compact_store() > zram = dev_to_zram(dev); > idr_for_each(zram_remove_cb > zram_remove > ... > kfree(zram) > down_read(&zram->init_lock); > > CPU 1 CPU 2 > hot_remove_store > compact_store() > zram = dev_to_zram(dev); > zram_remove > kfree(zram) > down_read(&zram->init_lock); > > To ensure the private data pointer is valid we could use bdget() / bdput() > in between access, however that would mean doing that in all sysfs > reads/stores on the driver. Instead a generic solution for all drivers > is to ensure the device kobject is still valid and also the bus, if > a bus is present. > > This issue does not fix a known crash, however this race was > spotted by Minchan Kim through code inspection upon code review > of another zram patch. > > Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx> > --- > drivers/base/base.h | 2 ++ > drivers/base/bus.c | 4 ++-- > drivers/base/core.c | 42 ++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 42 insertions(+), 6 deletions(-) Please make this an independent patch of the zram mess and I will be glad to consider it for the driver core tree then. thanks, greg k-h
