Date: Mon, 9 May 2005 22:20:40 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Warner Losh <imp@bsdimp.com>, freebsd-usb@freebsd.org Subject: Re: usb/80829: possible panic when loading USB-modules Message-ID: <200505092220.42970.hselasky@c2i.net> In-Reply-To: <20050509.110153.78761402.imp@bsdimp.com> References: <200505091849.15420.hselasky@c2i.net> <20050509.110153.78761402.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 09 May 2005 19:01, you wrote:
> > There is a special mechanism where probe/attach can clear an entry in the
> > array pointed to by "uaa->ifaces". The existing USB-driver allocates the
> > "uaa" in memory, but the "uaa->ifaces" is still on the stack ! This is
> > going to cause a panic for some devices when loaded as a module.
> >
> > usbd_status
> > usbd_probe_and_attach(device_ptr_t parent, usbd_device_handle dev,
> > int port, int addr)
> >
> > ...
> > usbd_interface_handle ifaces[256]; /* 256 is the absolute max */
> >
> > ...
> > uaa.ifaces = ifaces;
>
> Good catch!
>
> > Allocate "ifaces" structure in memory, and make sure it gets freed, or
> > revert everything back to stack, which is way simpler!
>
> Can't go back to the stack method.
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?
-- HPS
/* "usbd_probe_and_attach()" is called
* from "usbd_new_device()" and "uhub_explore()"
*/
usbd_status
usbd_probe_and_attach(struct device *parent, int port, struct usbd_port *up)
{
struct usb_attach_arg uaa;
struct usbd_device *udev = up->device;
struct device *bdev = NULL;
usbd_status err = 0;
int config;
int i;
#if 0
up->refcount = usb_driver_added_refcount;
#endif
if(udev == NULL)
{
PRINTF(("%s: port %d has no device\n",
device_get_nameunit(parent), port));
return (USBD_INVAL);
}
/* probe and attach */
uaa.device = udev;
uaa.port = port;
uaa.configno = UHUB_UNK_CONFIGURATION;
uaa.vendor = UGETW(udev->ddesc.idVendor);
uaa.product = UGETW(udev->ddesc.idProduct);
uaa.release = UGETW(udev->ddesc.bcdDevice);
/* "udev->probed" has four states:
*
* 0: nothing found (default value)
* 1: probed specific and found
* 2: probed iface and found
* 3: probed generic and found
*/
if((udev->probed == 1) ||
(udev->probed == 3))
{
/* nothing more to probe */
goto done;
}
bdev = device_add_child(parent, NULL, -1);
if(!bdev)
{
device_printf(udev->bus->bdev,
"Device creation failed\n");
err = USBD_INVAL;
goto done;
}
device_set_ivars(bdev, &uaa);
device_quiet(bdev);
if(udev->probed == 0)
{
/* first try with device specific drivers */
PRINTF(("trying device specific drivers\n"));
if(device_probe_and_attach(bdev) == 0)
{
device_set_ivars(bdev, NULL); /* no longer accessible
*/
udev->subdevs[0] = bdev;
udev->probed = 1;
bdev = 0;
goto done;
}
PRINTF(("no device specific driver found; "
"looping over %d configurations\n",
udev->ddesc.bNumConfigurations));
}
/* next try with interface drivers
* (in decremental order so
* that config 0 is set last,
* in case of failure!)
*/
if((udev->probed == 0) ||
(udev->probed == 2))
{
config = udev->ddesc.bNumConfigurations;
while(config--)
{
struct usbd_interface *iface;
/* only set config index the first
* time the devices are probed
*/
if(udev->probed == 0)
{
err = usbd_set_config_index(udev, config, 1);
if(err)
{
#ifdef USB_DEBUG
PRINTF(("%s: port %d, set config at addr %d
failed, "
"error=%s\n",
device_get_nameunit(parent),
port, udev->address,
usbd_errstr(err)));
#else
device_printf(parent,
"port %d, set config at addr %d
failed\n",
port, udev->address);
#endif
goto done;
}
}
/* ``bNumInterface'' is checked by ``usbd_set_config_index()''
*/
uaa.configno = udev->cdesc->bConfigurationValue;
uaa.ifaces_start = &udev->ifaces[0];
uaa.ifaces_end = &udev->ifaces[udev->cdesc->bNumInterface];
#ifdef USB_COMPAT_OLD
uaa.nifaces = udev->cdesc->bNumInterface;
for(i = 0; i < uaa.nifaces; i++)
{
if(udev->ifaces_probed[i / 8] & (1 << (i % 8)))
uaa.ifaces[i] = NULL;
else
uaa.ifaces[i] = &udev->ifaces[i];
}
#endif
for(iface = uaa.ifaces_start;
iface < uaa.ifaces_end;
iface++)
{
uaa.iface = iface;
uaa.iface_index = (i = (iface - &udev->ifaces[0]));
if(uaa.iface_index >= (sizeof(udev->subdevs)/
sizeof(udev->subdevs[0])))
{
device_printf(udev->bus->bdev,
"Too many subdevices\n");
break;
}
#ifdef USB_COMPAT_OLD
if(uaa.ifaces[i])
#endif
if((udev->subdevs[i] == NULL) &&
(device_probe_and_attach(bdev) == 0))
{
device_set_ivars(bdev, NULL); /* no longer
accessible */
udev->subdevs[i] = bdev;
udev->probed = 2;
bdev = 0;
/* create another child for the next iface [if
any] */
bdev = device_add_child(parent, NULL, -1);
if(!bdev)
{
device_printf(udev->bus->bdev,
"Device creation
failed\n");
break; /* need to update
"iface_probed" */
}
device_set_ivars(bdev, &uaa);
device_quiet(bdev);
}
}
if(udev->probed == 2)
{
#ifdef USB_COMPAT_OLD
uaa.nifaces = udev->cdesc->bNumInterface;
for(i = 0; i < uaa.nifaces; i++)
{
/* mark ifaces that should
* not be probed
*/
if(uaa.ifaces[i] == NULL)
{
udev->ifaces_probed[i / 8] |=
(1 << (i % 8));
}
}
#endif
break;
}
}
}
if(udev->probed == 0)
{
/* config index 0 should be set */
PRINTF(("no interface drivers found\n"));
/* finally try the generic driver */
uaa.iface = NULL;
uaa.iface_index = 0;
uaa.ifaces_start = NULL;
uaa.ifaces_end = NULL;
uaa.usegeneric = 1;
uaa.configno = UHUB_UNK_CONFIGURATION;
if(device_probe_and_attach(bdev) == 0)
{
device_set_ivars(bdev, NULL); /* no longer accessible
*/
udev->subdevs[0] = bdev;
udev->probed = 3;
bdev = 0;
goto done;
}
/*
* Generic attach failed.
* The device is left as it is.
* It has no driver, but is fully operational.
*/
PRINTF(("generic attach failed\n"));
}
done:
if(bdev)
{
/* remove the last created child; it is unused */
device_delete_child(parent, bdev);
}
return err;
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200505092220.42970.hselasky>