usb/80829: possible panic when loading USB-modules

Hans Petter Selasky hselasky at c2i.net
Tue May 10 02:33:31 PDT 2005


On Monday 09 May 2005 22:45, Warner Losh wrote:
> > I'm planning to use the stack method and have usbd_probe_and_attach()
> > called again from uhub_explore() via a call similar to usb_explore().
> > Leaving devices after usbd_probe_and_attach() has returned, will _not_
> > work, except for generic or specific USB drivers. It will not work for
> > "ums", "ukbd" and so on, because a device can have more than one
> > configuration, and it is not sure that the right configuration index is
> > set when usbd_probe_and_attach() returns after the generic probe. Have a
> > look at this (almost finished):
> >
> > Implement a new state-variable "udev->probed" and a refcount in
> > "usbd_port" to limit the calling of "usbd_probe_and_attach()".
> >
> > Any comments?
>
> That can't work.  The problem is that if there's no driver loaded, the
> device_t sticks around (and must stick around).  

My idea is to not have any device_t stick around. The only reason that the 
device_t must stick around, in the existing driver, is that they will get 
probed via "bus_generic_driver_added" when a new driver is added.

So change:
        DEVMETHOD(bus_driver_added, bus_generic_driver_added),

back into:
        DEVMETHOD(bus_driver_added, uhub_driver_added),

And have "uhub_driver_added" call , say, "usbd_probe_and_attach_all()", that 
will create device_t again and probe/attach, through "uhub_explore" and 
"usb_event_thread", and free if no device present. And if this is done right, 
it will not conflict with device removal! If you use 
"bus_generic_driver_added", won't there be a race condition where a device 
can be probed/attached at the same time it is detached, because probe/attach 
can call USB-functions that can sleep, like usbd_do_request()? When sleeping 
no lock is held.

> When the driver is 
> then loaded, we look at all the unattached devices again and at that
> point the memory that's reference has to be stable. That's why I made 
> the change in the first place.
>

> Is there actually problem with any current usage of uaa.ifaces? I
> can see that the pointer to the ifaces[] array on the stack can get
> stored in the malloc'd `uaap' structure, but I couldn't see anywhere
> that actually references the ifaces[] array after usbd_probe_and_attach()
> returns - I'm probably missing something simple though.

When you are loading an USB-module, because device_t is still sticking around, 
probe/attach can be called outside of "usbd_probe_and_attach()", actually 
from "bus_generic_driver_added". But in my opinion the uaaxxx structures 
should not be available after probe/attach, and that probe/attach should not 
be called outside of "usbd_probe_and_attach()".

--HPS


More information about the freebsd-usb mailing list