lkml.org 
[lkml]   [2006]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: race in sysfs between sysfs_remove_file() and read()/write() #2
    On Mon, Dec 04, 2006 at 07:38:00AM +0100, Oliver Neukum wrote:
    > Am Montag, 4. Dezember 2006 05:43 schrieb Maneesh Soni:
    > > On Fri, Dec 01, 2006 at 11:43:06PM +0100, Oliver Neukum wrote:
    > > > Hi,
    > > >
    > > > Alan Stern has discovered a race in sysfs, whereby driver callbacks could be
    > > > called after sysfs_remove_file() has run. The attached patch should fix it.
    > > >
    > > > It introduces a new data structure acting as a collection of all sysfs_buffers
    > > > associated with an attribute. Upon removal of an attribute the buffers are
    > > > marked orphaned and IO on them returns -ENODEV. Thus sysfs_remove_file()
    > > > makes sure that sysfs won't bother a driver after that call, making it safe
    > > > to free the associated data structures and to unload the driver.
    > > >
    > > > Regards
    > > > Oliver
    > >
    > > 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.

    Greg?

    Thanks
    Maneesh
    -
    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/

    \
     
     \ /
      Last update: 2006-12-04 14:07    [W:0.024 / U:0.708 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site