usb/80829: possible panic when loading USB-modules

Hans Petter Selasky hselasky at c2i.net
Mon May 9 13:20:08 PDT 2005


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;
}


More information about the freebsd-usb mailing list