Messages in this thread | | | From | Oliver Neukum <> | Subject | Re: race in sysfs between sysfs_remove_file() and read()/write() #2 | Date | Mon, 4 Dec 2006 14:58:50 +0100 |
| |
Am Montag, 4. Dezember 2006 14:04 schrieb Maneesh Soni: > > > Hi Oliver, > > > > > > Thanks for the explaining the patch but some description about the race > > > would also help here. At the least the callpath to the race would be useful. > > > > > > > > > Thanks > > > Maneesh > > > > We have code like this: > > static void tv_disconnect(struct usb_interface *interface) > > { > > struct trancevibrator *dev; > > > > dev = usb_get_intfdata (interface); > > device_remove_file(&interface->dev, &dev_attr_speed); > > usb_set_intfdata(interface, NULL); > > usb_put_dev(dev->udev); > > kfree(dev); > > } > > > > This has a race: > > > > CPU A CPU B > > open sysfs > > device_remove_file > > kfree > > reading attr > > > > We cannot do refcounting as sysfs doesn't export open/close. Therefore > > we must be sure that device_remove_file() makes sure that sysfs will > > leave a driver alone after the return of device_remove_file(). Currently > > open will fail, but IO on an already opened file will work. The patch makes > > sure it will fail with -ENODEV without calling into the driver, which may > > indeed be already unloaded. > > > > Regards > > Oliver > > hmm, I guess Greg has to say the final word. The question is either to fail > the IO (-ENODEV) or fail the file removal (-EBUSY). If we are not going to > fail the removal then your patch is the way to go.
Failing the removal is problematic. This happens in the disconnect() code path, which cannot fail in a benign way. Plus, if we do so, the module refcounting in sysfs is incorrect, that is too early.
Regards Oliver - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |