linux-kernel - Re: race in sysfs between sysfs_remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200612041458.50753.oliver@neukum.org>
Date:	Mon, 4 Dec 2006 14:58:50 +0100
From:	Oliver Neukum <oliver@...kum.org>
To:	maneesh@...ibm.com
Cc:	gregkh@...e.com, Alan Stern <stern@...land.harvard.edu>,
	linux-usb-devel@...ts.sourceforge.net,
	kernel list <linux-kernel@...r.kernel.org>
Subject: Re: race in sysfs between sysfs_remove_file() and read()/write() #2

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/