Date: Tue, 10 May 2005 11:34:13 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Warner Losh <imp@bsdimp.com> Cc: freebsd-usb@freebsd.org Subject: Re: usb/80829: possible panic when loading USB-modules Message-ID: <200505101134.14381.hselasky@c2i.net> In-Reply-To: <20050509.144546.74694206.imp@bsdimp.com> References: <200505091849.15420.hselasky@c2i.net> <200505092220.42970.hselasky@c2i.net> <20050509.144546.74694206.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200505101134.14381.hselasky>