d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Resolve a LOR after r246616. Protect control requests using the USB device

Browse Source 
enumeration lock. Make sure all callers of usbd_enum_lock() check the return
value. Remove the control transfer specific lock. Bump the FreeBSD version
number, hence external USB modules may need to be recompiled due to a USB
device structure change.

MFC after:	1 week
This commit is contained in:
hselasky 2013-02-13 12:35:17 +00:00
parent 6133fc8dbb
commit a40dc60f62
9 changed files with 56 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
sys/dev/usb/controller/usb_controller.c
View File

@ -427,6 +427,7 @@ usb_bus_suspend(struct usb_proc_msg *pm)
struct usb_bus *bus;
struct usb_device *udev;
usb_error_t err;
uint8_t do_unlock;
bus = ((struct usb_bus_msg *)pm)->bus;
udev = bus->devices[USB_ROOT_HUB_ADDR];
@ -447,7 +448,7 @@ usb_bus_suspend(struct usb_proc_msg *pm)
bus_generic_shutdown(bus->bdev);
usbd_enum_lock(udev);
do_unlock = usbd_enum_lock(udev);
err = usbd_set_config_index(udev, USB_UNCONFIG_INDEX);
if (err)
@ -464,7 +465,8 @@ usb_bus_suspend(struct usb_proc_msg *pm)
if (bus->methods->set_hw_power_sleep != NULL)
(bus->methods->set_hw_power_sleep) (bus, USB_HW_POWER_SUSPEND);
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_BUS_LOCK(bus);
}
@ -480,6 +482,7 @@ usb_bus_resume(struct usb_proc_msg *pm)
struct usb_bus *bus;
struct usb_device *udev;
usb_error_t err;
uint8_t do_unlock;
bus = ((struct usb_bus_msg *)pm)->bus;
udev = bus->devices[USB_ROOT_HUB_ADDR];
@ -489,7 +492,7 @@ usb_bus_resume(struct usb_proc_msg *pm)
USB_BUS_UNLOCK(bus);
usbd_enum_lock(udev);
do_unlock = usbd_enum_lock(udev);
#if 0
DEVMETHOD(usb_take_controller, NULL); /* dummy */
#endif
@ -523,7 +526,8 @@ usb_bus_resume(struct usb_proc_msg *pm)
"attach root HUB\n");
}
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_BUS_LOCK(bus);
}
@ -539,6 +543,7 @@ usb_bus_shutdown(struct usb_proc_msg *pm)
struct usb_bus *bus;
struct usb_device *udev;
usb_error_t err;
uint8_t do_unlock;
bus = ((struct usb_bus_msg *)pm)->bus;
udev = bus->devices[USB_ROOT_HUB_ADDR];
@ -550,7 +555,7 @@ usb_bus_shutdown(struct usb_proc_msg *pm)
bus_generic_shutdown(bus->bdev);
usbd_enum_lock(udev);
do_unlock = usbd_enum_lock(udev);
err = usbd_set_config_index(udev, USB_UNCONFIG_INDEX);
if (err)
@ -567,7 +572,8 @@ usb_bus_shutdown(struct usb_proc_msg *pm)
if (bus->methods->set_hw_power_sleep != NULL)
(bus->methods->set_hw_power_sleep) (bus, USB_HW_POWER_SHUTDOWN);
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_BUS_LOCK(bus);
}

11
sys/dev/usb/usb_dev.c
View File

@ -218,10 +218,10 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
mtx_unlock(&usb_ref_lock);
/*
* We need to grab the sx-lock before grabbing the
* FIFO refs to avoid deadlock at detach!
* We need to grab the enumeration SX-lock before
* grabbing the FIFO refs to avoid deadlock at detach!
*/
usbd_enum_lock(cpd->udev);
crd->do_unlock = usbd_enum_lock(cpd->udev);
mtx_lock(&usb_ref_lock);
@ -282,9 +282,10 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
return (0);
error:
if (crd->is_uref) {
if (crd->do_unlock)
usbd_enum_unlock(cpd->udev);
if (crd->is_uref) {
if (--(cpd->udev->refcount) == 0) {
cv_signal(&cpd->udev->ref_cv);
}
@ -336,7 +337,7 @@ usb_unref_device(struct usb_cdev_privdata *cpd,
DPRINTFN(2, "cpd=%p is_uref=%d\n", cpd, crd->is_uref);
if (crd->is_uref)
if (crd->do_unlock)
usbd_enum_unlock(cpd->udev);
mtx_lock(&usb_ref_lock);

1
sys/dev/usb/usb_dev.h
View File

@ -84,6 +84,7 @@ struct usb_cdev_refdata {
uint8_t is_write; /* location has write access */
uint8_t is_uref; /* USB refcount decr. needed */
uint8_t is_usbfs; /* USB-FS is active */
uint8_t do_unlock; /* USB enum unlock needed */
};
struct usb_fs_privdata {

4
sys/dev/usb/usb_device.c
View File

@ -1545,9 +1545,6 @@ usb_alloc_device(device_t parent_dev, struct usb_bus *bus,
if (udev == NULL) {
return (NULL);
}
/* initialise our SX-lock */
sx_init_flags(&udev->ctrl_sx, "USB device SX lock", SX_DUPOK);
/* initialise our SX-lock */
sx_init_flags(&udev->enum_sx, "USB config SX lock", SX_DUPOK);
sx_init_flags(&udev->sr_sx, "USB suspend and resume SX lock", SX_NOWITNESS);
@ -2119,7 +2116,6 @@ usb_free_device(struct usb_device *udev, uint8_t flag)
&udev->cs_msg[0], &udev->cs_msg[1]);
USB_BUS_UNLOCK(udev->bus);
sx_destroy(&udev->ctrl_sx);
sx_destroy(&udev->enum_sx);
sx_destroy(&udev->sr_sx);

1
sys/dev/usb/usb_device.h
View File

@ -181,7 +181,6 @@ union usb_device_scratch {
struct usb_device {
struct usb_clear_stall_msg cs_msg[2]; /* generic clear stall
* messages */
struct sx ctrl_sx;
struct sx enum_sx;
struct sx sr_sx;
struct mtx device_mtx;

20
sys/dev/usb/usb_handle_request.c
View File

@ -149,6 +149,7 @@ usb_handle_set_config(struct usb_xfer *xfer, uint8_t conf_no)
{
struct usb_device *udev = xfer->xroot->udev;
usb_error_t err = 0;
uint8_t do_unlock;
/*
* We need to protect against other threads doing probe and
@ -156,7 +157,8 @@ usb_handle_set_config(struct usb_xfer *xfer, uint8_t conf_no)
*/
USB_XFER_UNLOCK(xfer);
usbd_enum_lock(udev);
/* Prevent re-enumeration */
do_unlock = usbd_enum_lock(udev);
if (conf_no == USB_UNCONFIG_NO) {
conf_no = USB_UNCONFIG_INDEX;
@ -179,7 +181,8 @@ usb_handle_set_config(struct usb_xfer *xfer, uint8_t conf_no)
goto done;
}
done:
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_XFER_LOCK(xfer);
return (err);
}
@ -221,6 +224,7 @@ usb_handle_iface_request(struct usb_xfer *xfer,
int error;
uint8_t iface_index;
uint8_t temp_state;
uint8_t do_unlock;
if ((req.bmRequestType & 0x1F) == UT_INTERFACE) {
iface_index = req.wIndex[0]; /* unicast */
@ -234,7 +238,8 @@ usb_handle_iface_request(struct usb_xfer *xfer,
*/
USB_XFER_UNLOCK(xfer);
usbd_enum_lock(udev);
/* Prevent re-enumeration */
do_unlock = usbd_enum_lock(udev);
error = ENXIO;
@ -350,17 +355,20 @@ usb_handle_iface_request(struct usb_xfer *xfer,
goto tr_stalled;
}
tr_valid:
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_XFER_LOCK(xfer);
return (0);
tr_short:
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_XFER_LOCK(xfer);
return (USB_ERR_SHORT_XFER);
tr_stalled:
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
USB_XFER_LOCK(xfer);
return (USB_ERR_STALLED);
}

13
sys/dev/usb/usb_hub.c
View File

@ -243,7 +243,9 @@ uhub_explore_sub(struct uhub_softc *sc, struct usb_port *up)
/* check if device should be re-enumerated */
if (child->flags.usb_mode == USB_MODE_HOST) {
usbd_enum_lock(child);
uint8_t do_unlock;
do_unlock = usbd_enum_lock(child);
if (child->re_enumerate_wait) {
err = usbd_set_config_index(child,
USB_UNCONFIG_INDEX);
@ -262,7 +264,8 @@ uhub_explore_sub(struct uhub_softc *sc, struct usb_port *up)
child->re_enumerate_wait = 0;
err = 0;
}
usbd_enum_unlock(child);
if (do_unlock)
usbd_enum_unlock(child);
}
/* check if probe and attach should be done */
@ -716,6 +719,7 @@ uhub_explore(struct usb_device *udev)
usb_error_t err;
uint8_t portno;
uint8_t x;
uint8_t do_unlock;
hub = udev->hub;
sc = hub->hubsoftc;
@ -737,7 +741,7 @@ uhub_explore(struct usb_device *udev)
* Make sure we don't race against user-space applications
* like LibUSB:
*/
usbd_enum_lock(udev);
do_unlock = usbd_enum_lock(udev);
for (x = 0; x != hub->nports; x++) {
up = hub->ports + x;
@ -817,7 +821,8 @@ uhub_explore(struct usb_device *udev)
up->restartcnt = 0;
}
usbd_enum_unlock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
/* initial status checked */
sc->sc_flags |= UHUB_FLAG_DID_EXPLORE;

30
sys/dev/usb/usb_request.c
View File

@ -389,9 +389,8 @@ usbd_get_hr_func(struct usb_device *udev)
* than 30 seconds is treated like a 30 second timeout. This USB stack
* does not allow control requests without a timeout.
*
* NOTE: This function is thread safe. All calls to
* "usbd_do_request_flags" will be serialised by the use of an
* internal "sx_lock".
* NOTE: This function is thread safe. All calls to "usbd_do_request_flags"
* will be serialized by the use of the USB device enumeration lock.
*
* Returns:
* 0: Success
@ -415,7 +414,7 @@ usbd_do_request_flags(struct usb_device *udev, struct mtx *mtx,
uint16_t length;
uint16_t temp;
uint16_t acttemp;
uint8_t enum_locked;
uint8_t do_unlock;
if (timeout < 50) {
/* timeout is too small */
@ -427,8 +426,6 @@ usbd_do_request_flags(struct usb_device *udev, struct mtx *mtx,
}
length = UGETW(req->wLength);
enum_locked = usbd_enum_is_locked(udev);
DPRINTFN(5, "udev=%p bmRequestType=0x%02x bRequest=0x%02x "
"wValue=0x%02x%02x wIndex=0x%02x%02x wLength=0x%02x%02x\n",
udev, req->bmRequestType, req->bRequest,
@ -458,18 +455,17 @@ usbd_do_request_flags(struct usb_device *udev, struct mtx *mtx,
mtx_assert(mtx, MA_NOTOWNED);
}
/*
* Grab the USB device enumeration SX-lock serialization is
* achieved when multiple threads are involved:
*/
do_unlock = usbd_enum_lock(udev);
/*
* We need to allow suspend and resume at this point, else the
* control transfer will timeout if the device is suspended!
*/
if (enum_locked)
usbd_sr_unlock(udev);
/*
* Grab the default sx-lock so that serialisation
* is achieved when multiple threads are involved:
*/
sx_xlock(&udev->ctrl_sx);
usbd_sr_unlock(udev);
hr_func = usbd_get_hr_func(udev);
@ -713,10 +709,10 @@ usbd_do_request_flags(struct usb_device *udev, struct mtx *mtx,
USB_XFER_UNLOCK(xfer);
done:
sx_xunlock(&udev->ctrl_sx);
usbd_sr_lock(udev);
if (enum_locked)
usbd_sr_lock(udev);
if (do_unlock)
usbd_enum_unlock(udev);
if ((mtx != NULL) && (mtx != &Giant))
mtx_lock(mtx);

2
sys/sys/param.h
View File

@ -58,7 +58,7 @@
* in the range 5 to 9.
*/
#undef __FreeBSD_version
#define __FreeBSD_version 1000027 /* Master, propagated to newvers */
#define __FreeBSD_version 1000028 /* Master, propagated to newvers */
/*
* __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,