Date: Tue, 10 May 2005 19:51:51 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: FreeBSD-gnats-submit@FreeBSD.org Subject: usb/80862: USB locking issues Message-ID: <200505101951.52740.hselasky@c2i.net> Resent-Message-ID: <200505101800.j4AI0O3F087777@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 80862
>Category: usb
>Synopsis: USB locking issues
>Confidential: no
>Severity: critical
>Priority: medium
>Responsible: freebsd-usb
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Tue May 10 18:00:24 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: HPS
>Release: FreeBSD 6.0-CURRENT i386
>Organization:
>Environment:
System: FreeBSD 6.0-CURRENT FreeBSD 6.0-CURRENT #45: Mon Mar 21 15:40:17 CET
2005 root@:/usr/obj/usr/src/sys/custom i386
>Description:
I'm not sure if "sysctl" calls are under Giant, but "uhub_child_xxxx()" are
called from sysctl context, and should lock Giant or assert Giant. Secondly
these calls should not sleep, because then Giant will be exited, and the
"usb_event_thread()" can detach the USB-device meanwhile.
>How-To-Repeat:
>Fix:
In the file "/sys/dev/usb/uhub.c":
uhub_child_location_str()
{
...
mtx_lock(&Giant); /* missing */
...
mtx_unlock(&Giant);
}
uhub_child_pnpinfo_str()
{
...
mtx_lock(&Giant); /* missing */
This must be moved into "usbd_new_device()", because it can sleep:
serial[0] = '\0';
(void)usbd_get_string(dev, dev->ddesc.iSerialNumber, &serial[0]);
Instead something like:
strcpy(&serial[0], &dev->serial[0]);
must be used. Or just replace "serial" with "dev->serial".
...
mtx_unlock(&Giant);
}
usbd_new_device()
{
...
udev->serial[0] = '\0';
(void)usbd_get_string(dev, udev->ddesc.iSerialNumber, &udev->serial[0]
/* nice with a length parameter here to limit the
length of the serial number */);
/* format checking */
for(i = 0;;i++)
{
if(udev->serial[i] == '\0') break;
if(udev->serial[i] == '\"') udev->serial[i] = ' ';
if(udev->serial[i] == '\n') udev->serial[i] = ' ';
}
...
}
Something else:
Should allow root hub to detach:
static driver_t uhubroot_driver =
{
...
DEVMETHOD(device_detach, uhub_detach),
...
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200505101951.52740.hselasky>