On Mon, 30 Nov 2009, David Vrabel wrote: > Alan Stern wrote: > > Inaky and David: > > > > Looking through usb_authorize_device() and usb_deauthorize_device(), > > those two routines appear to have several problems: > > I think the whole concept doesn't really fit with how the Wireless USB > protocol works. I'd like to see. > > * The device's "authorized" flag renamed to "authenticated" (similarly > renaming the HCD's authorized_default flag to > "devices_require_authentication"). > > * The "authorized" sysfs file renamed to "authenticated" and made read-only. > > * The "authorized_default" sysfs file removed. > > * usb_deauthorize_device() removed. > > * usb_authorize_device() renamed to usb_device_authenticated() and > called from the WUSB core when a device is authenticated. There was a similar confusion at the time all this stuff was originally merged. Inaky might have a different take on this, but as I recall he explained that authentication and authorization are two separate and more-or-less independent things. Authentication is the process of determining that a USB device really is what it claims to be, and authorization is the process of giving the system permission to use the device. If you want to allow the system to use any device that has been authenticated, then the authorization stuff isn't needed. Inaky had in mind that system managers might want to prohibit use of certain USB devices for administrative reasons, regardless of authentication. > It would also be nice to get the core to call usb_device_authenticated() > for newly connected wired devices. The should reduce some code duplication. > > > > > usb_authorize_device() calls usb_autoresume_device() but > > not usb_autosuspend_device(). It this a simple oversight? > > I guess. I don't understand how this autosuspend thing is supposed to > work though. It sure looks like an oversight. In brief, usb_autoresume_device() increments a usage counter and resumes the device if it is currently suspended. usb_autosuspend_device() decrements the usage counter and tries to suspend the device if the counter drops to 0. > > usb_deauthorize_device() frees a lot of refcounted data > > structures without regard to the refcounts. Why doesn't > > it leave them to be deallocated in the normal way? > > > > usb_deauthorize_device() leaks several strings. Oddly > > enough, usb_authorize_device() handles them correctly. But > > it races with the strings' show routines in sysfs.c, > > because those routines don't acquire the device lock. > > Shouldn't the show routines take the device lock then? That's the way to fix the problem. At the time those routines were written, nothing else modified the strings. Hence there was no need for them to take the device lock. > > Does either of you want to fix these issues? > > I don't have time until Jan/Feb next year at the earliest. I can make the changes. I just wanted to check first and make sure that there wouldn't be any objection. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
