On Tue, Jun 22, 2021 at 09:44:02AM -0700, Luis Chamberlain wrote: > On Tue, Jun 22, 2021 at 09:46:26AM +0200, Greg KH wrote: > > 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. > > What do you mean by making it independent? > > The patch does not depend on the zram changes, and so, this can > be merged separately as-is. Great, then make it a 1/1 patch. Putting it as patch 3 here means I can not take it as our tools pull in the full series. thanks, greg k-h
