On Thu, 29 Jun 2006, Franck Bui-Huu wrote: > ok it seems you did the job for usbdev_open(). But one thing I fail to > understand is: > > """ > Yet another problem arises because the same file_operations structure is > accessible through both the /proc/bus/usb/* and /dev/usb/usbdev* file > nodes. Even when one of them has been removed, it's still possible for > userspace to open the other. So simple locking around the individual > remove routines is insufficient; we need to lock the entire > usb_notify_remove_device notifier chain. > """
It's simple enough. On my old system, /proc/bus/usb/001/002 and /dev/usbdev1.2 are two different nodes in the filesystem that both end up using usbdev_open. When the device gets unplugged from the computer we want to remove those file nodes and we also want to make sure that users can't open the files while they're being removed. The existing locking will prevent users from opening /dev/usbdev1.2 while it is being removed, but it won't prevent users from opening /proc/bus/usb/001/002 while /dev/usbdev1.2 is being removed. > One other question is does usbdev_remove() really acquire the device > lock ? No; it is called with the lock already acquired. > > I didn't notice that it was used in usbdev_lseek and there doesn't seem to > > be any reason for it to be there. Nor does there seem to be any reason > > for it in usb_device_lseek. > > > > The code in usb_device_poll probably should be revised in view of the > > changes in my patch. You might want to look at that. But you're right, > > it shouldn't be using the BKL. > > > > are we talking about the same thing here ? I was speaking about > usb_device_poll() in devices.c, and it seems you're talking about > usbdev_poll() which is in devio.c. I don't remember which one I was talking about. :-) The prudent course would be to review both routines. Though it's not clear to me why we should need two... Let's face it, usbfs is kind of a mess. > > The use of BKL in usb-skeleton and possibly some of the other drivers > > follows a simple pattern. It is there to prevent races between open() and > > disconnect(). The idea is that the kernel automatically acquires the BKL > > whenever a character device node is opened, so by taking the lock in the > > disconnect routine we avoid the problem of people opening a device file > > as it is being unregistered. > > > > can we just use the device's lock for that purpose ? No, for a rather interesting reason. The open() routine needs to hold the lock during the time when the device is being looked up. (For usb-skeleton, this is during the call to usb_find_interface.) But you can't acquire the device lock until you know _which_ device to lock, i.e., until after you know the result of the lookup! Alan Stern Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
