aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHans Petter Selasky <hselasky@FreeBSD.org>2014-06-08 20:10:29 +0000
committerHans Petter Selasky <hselasky@FreeBSD.org>2014-06-08 20:10:29 +0000
commitd64e9217c40746d8e4f8172457226b9abfe11be5 (patch)
tree6b6fd9094c2096380007cac3ef2c1b6c0201594f
parent3345d73ca8033de27169a478810f39a7a8f161ec (diff)
downloadsrc-d64e9217c40746d8e4f8172457226b9abfe11be5.tar.gz
src-d64e9217c40746d8e4f8172457226b9abfe11be5.zip
Resolve a deadlock setting the USB configuration index from userspace
on USB HUBs by moving the code into the USB explore threads. The deadlock happens because child devices of the USB HUB don't have the expected reference count when called from outside the explore thread. Only the HUB device itself, which the IOCTL interface locks, gets the correct reference count. MFC after: 3 days
Notes
Notes: svn path=/head/; revision=267240
-rw-r--r--sys/dev/usb/controller/usb_controller.c8
-rw-r--r--sys/dev/usb/usb_dev.c11
-rw-r--r--sys/dev/usb/usb_device.h2
-rw-r--r--sys/dev/usb/usb_generic.c20
-rw-r--r--sys/dev/usb/usb_hub.c162
-rw-r--r--sys/dev/usb/usb_hub.h1
-rw-r--r--sys/dev/usb/usbdi.h2
7 files changed, 133 insertions, 73 deletions
diff --git a/sys/dev/usb/controller/usb_controller.c b/sys/dev/usb/controller/usb_controller.c
index 58b121bcddd7..6df543030d94 100644
--- a/sys/dev/usb/controller/usb_controller.c
+++ b/sys/dev/usb/controller/usb_controller.c
@@ -367,7 +367,13 @@ usb_bus_explore(struct usb_proc_msg *pm)
if (bus->no_explore != 0)
return;
- if (udev && udev->hub) {
+ if (udev != NULL) {
+ USB_BUS_UNLOCK(bus);
+ uhub_explore_handle_re_enumerate(udev);
+ USB_BUS_LOCK(bus);
+ }
+
+ if (udev != NULL && udev->hub != NULL) {
if (bus->do_probe) {
bus->do_probe = 0;
diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c
index e4471aee0b74..6ad02a7d035d 100644
--- a/sys/dev/usb/usb_dev.c
+++ b/sys/dev/usb/usb_dev.c
@@ -1116,9 +1116,14 @@ usb_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int fflag, struct thread*
usb_pause_mtx(NULL, hz / 128);
- if (usb_ref_device(cpd, &refs, 1 /* need uref */)) {
- err = ENXIO;
- goto done;
+ while (usb_ref_device(cpd, &refs, 1 /* need uref */)) {
+ if (usb_ref_device(cpd, &refs, 0)) {
+ /* device no longer exits */
+ err = ENXIO;
+ goto done;
+ }
+ usb_unref_device(cpd, &refs);
+ usb_pause_mtx(NULL, hz / 128);
}
}
diff --git a/sys/dev/usb/usb_device.h b/sys/dev/usb/usb_device.h
index 0763663fec6f..4bda2a476aba 100644
--- a/sys/dev/usb/usb_device.h
+++ b/sys/dev/usb/usb_device.h
@@ -228,6 +228,7 @@ struct usb_device {
uint8_t address; /* device addess */
uint8_t device_index; /* device index in "bus->devices" */
uint8_t controller_slot_id; /* controller specific value */
+ uint8_t next_config_index; /* used by USB_RE_ENUM_SET_CONFIG */
uint8_t curr_config_index; /* current configuration index */
uint8_t curr_config_no; /* current configuration number */
uint8_t depth; /* distance from root HUB */
@@ -241,6 +242,7 @@ struct usb_device {
#define USB_RE_ENUM_DONE 0
#define USB_RE_ENUM_START 1
#define USB_RE_ENUM_PWR_OFF 2
+#define USB_RE_ENUM_SET_CONFIG 3
uint8_t ifaces_max; /* number of interfaces present */
uint8_t endpoints_max; /* number of endpoints present */
diff --git a/sys/dev/usb/usb_generic.c b/sys/dev/usb/usb_generic.c
index 6d8b0f5bfce3..4ebd557a5182 100644
--- a/sys/dev/usb/usb_generic.c
+++ b/sys/dev/usb/usb_generic.c
@@ -616,24 +616,17 @@ ugen_set_config(struct usb_fifo *f, uint8_t index)
/* not possible in device side mode */
return (ENOTTY);
}
- if (f->udev->curr_config_index == index) {
- /* no change needed */
- return (0);
- }
+
/* make sure all FIFO's are gone */
/* else there can be a deadlock */
if (ugen_fs_uninit(f)) {
/* ignore any errors */
DPRINTFN(6, "no FIFOs\n");
}
- /* change setting - will free generic FIFOs, if any */
- if (usbd_set_config_index(f->udev, index)) {
- return (EIO);
- }
- /* probe and attach */
- if (usb_probe_and_attach(f->udev, USB_IFACE_INDEX_ANY)) {
+
+ if (usbd_start_set_config(f->udev, index) != 0)
return (EIO);
- }
+
return (0);
}
@@ -970,11 +963,6 @@ ugen_re_enumerate(struct usb_fifo *f)
DPRINTFN(6, "device mode\n");
return (ENOTTY);
}
- if (udev->parent_hub == NULL) {
- /* the root HUB cannot be re-enumerated */
- DPRINTFN(6, "cannot reset root HUB\n");
- return (EINVAL);
- }
/* make sure all FIFO's are gone */
/* else there can be a deadlock */
if (ugen_fs_uninit(f)) {
diff --git a/sys/dev/usb/usb_hub.c b/sys/dev/usb/usb_hub.c
index 912723b00309..e9c9a42c3914 100644
--- a/sys/dev/usb/usb_hub.c
+++ b/sys/dev/usb/usb_hub.c
@@ -427,6 +427,86 @@ done:
return (retval);
}
+void
+uhub_explore_handle_re_enumerate(struct usb_device *child)
+{
+ uint8_t do_unlock;
+ usb_error_t err;
+
+ /* check if device should be re-enumerated */
+ if (child->flags.usb_mode != USB_MODE_HOST)
+ return;
+
+ do_unlock = usbd_enum_lock(child);
+ switch (child->re_enumerate_wait) {
+ case USB_RE_ENUM_START:
+ err = usbd_set_config_index(child,
+ USB_UNCONFIG_INDEX);
+ if (err != 0) {
+ DPRINTF("Unconfigure failed: %s: Ignored.\n",
+ usbd_errstr(err));
+ }
+ if (child->parent_hub == NULL) {
+ /* the root HUB cannot be re-enumerated */
+ DPRINTFN(6, "cannot reset root HUB\n");
+ err = 0;
+ } else {
+ err = usbd_req_re_enumerate(child, NULL);
+ }
+ if (err == 0)
+ err = usbd_set_config_index(child, 0);
+ if (err == 0) {
+ err = usb_probe_and_attach(child,
+ USB_IFACE_INDEX_ANY);
+ }
+ child->re_enumerate_wait = USB_RE_ENUM_DONE;
+ break;
+
+ case USB_RE_ENUM_PWR_OFF:
+ /* get the device unconfigured */
+ err = usbd_set_config_index(child,
+ USB_UNCONFIG_INDEX);
+ if (err) {
+ DPRINTFN(0, "Could not unconfigure "
+ "device (ignored)\n");
+ }
+ if (child->parent_hub == NULL) {
+ /* the root HUB cannot be re-enumerated */
+ DPRINTFN(6, "cannot set port feature\n");
+ err = 0;
+ } else {
+ /* clear port enable */
+ err = usbd_req_clear_port_feature(child->parent_hub,
+ NULL, child->port_no, UHF_PORT_ENABLE);
+ if (err) {
+ DPRINTFN(0, "Could not disable port "
+ "(ignored)\n");
+ }
+ }
+ child->re_enumerate_wait = USB_RE_ENUM_DONE;
+ break;
+
+ case USB_RE_ENUM_SET_CONFIG:
+ err = usbd_set_config_index(child,
+ child->next_config_index);
+ if (err != 0) {
+ DPRINTF("Configure failed: %s: Ignored.\n",
+ usbd_errstr(err));
+ } else {
+ err = usb_probe_and_attach(child,
+ USB_IFACE_INDEX_ANY);
+ }
+ child->re_enumerate_wait = USB_RE_ENUM_DONE;
+ break;
+
+ default:
+ child->re_enumerate_wait = USB_RE_ENUM_DONE;
+ break;
+ }
+ if (do_unlock)
+ usbd_enum_unlock(child);
+}
+
/*------------------------------------------------------------------------*
* uhub_explore_sub - subroutine
*
@@ -455,59 +535,7 @@ uhub_explore_sub(struct uhub_softc *sc, struct usb_port *up)
goto done;
}
- /* check if device should be re-enumerated */
-
- if (child->flags.usb_mode == USB_MODE_HOST) {
- uint8_t do_unlock;
-
- do_unlock = usbd_enum_lock(child);
- switch (child->re_enumerate_wait) {
- case USB_RE_ENUM_START:
- err = usbd_set_config_index(child,
- USB_UNCONFIG_INDEX);
- if (err != 0) {
- DPRINTF("Unconfigure failed: "
- "%s: Ignored.\n",
- usbd_errstr(err));
- }
- err = usbd_req_re_enumerate(child, NULL);
- if (err == 0)
- err = usbd_set_config_index(child, 0);
- if (err == 0) {
- err = usb_probe_and_attach(child,
- USB_IFACE_INDEX_ANY);
- }
- child->re_enumerate_wait = USB_RE_ENUM_DONE;
- err = 0;
- break;
-
- case USB_RE_ENUM_PWR_OFF:
- /* get the device unconfigured */
- err = usbd_set_config_index(child,
- USB_UNCONFIG_INDEX);
- if (err) {
- DPRINTFN(0, "Could not unconfigure "
- "device (ignored)\n");
- }
-
- /* clear port enable */
- err = usbd_req_clear_port_feature(child->parent_hub,
- NULL, child->port_no, UHF_PORT_ENABLE);
- if (err) {
- DPRINTFN(0, "Could not disable port "
- "(ignored)\n");
- }
- child->re_enumerate_wait = USB_RE_ENUM_DONE;
- err = 0;
- break;
-
- default:
- child->re_enumerate_wait = USB_RE_ENUM_DONE;
- break;
- }
- if (do_unlock)
- usbd_enum_unlock(child);
- }
+ uhub_explore_handle_re_enumerate(child);
/* check if probe and attach should be done */
@@ -2799,3 +2827,31 @@ usbd_start_re_enumerate(struct usb_device *udev)
usb_needs_explore(udev->bus, 0);
}
}
+
+/*-----------------------------------------------------------------------*
+ * usbd_start_set_config
+ *
+ * This function starts setting a USB configuration. This function
+ * does not need to be called BUS-locked. This function does not wait
+ * until the set USB configuratino is completed.
+ *------------------------------------------------------------------------*/
+usb_error_t
+usbd_start_set_config(struct usb_device *udev, uint8_t index)
+{
+ if (udev->re_enumerate_wait == USB_RE_ENUM_DONE) {
+ if (udev->curr_config_index == index) {
+ /* no change needed */
+ return (0);
+ }
+ udev->next_config_index = index;
+ udev->re_enumerate_wait = USB_RE_ENUM_SET_CONFIG;
+ usb_needs_explore(udev->bus, 0);
+ return (0);
+ } else if (udev->re_enumerate_wait == USB_RE_ENUM_SET_CONFIG) {
+ if (udev->next_config_index == index) {
+ /* no change needed */
+ return (0);
+ }
+ }
+ return (USB_ERR_PENDING_REQUESTS);
+}
diff --git a/sys/dev/usb/usb_hub.h b/sys/dev/usb/usb_hub.h
index dc8705a6b481..16430d9b29a7 100644
--- a/sys/dev/usb/usb_hub.h
+++ b/sys/dev/usb/usb_hub.h
@@ -75,5 +75,6 @@ void usb_bus_power_update(struct usb_bus *bus);
void usb_bus_powerd(struct usb_bus *bus);
void uhub_root_intr(struct usb_bus *, const uint8_t *, uint8_t);
usb_error_t uhub_query_info(struct usb_device *, uint8_t *, uint8_t *);
+void uhub_explore_handle_re_enumerate(struct usb_device *);
#endif /* _USB_HUB_H_ */
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index f4c6cf9ec941..09b0ca7b1b52 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -586,6 +586,8 @@ void usbd_m_copy_in(struct usb_page_cache *cache, usb_frlength_t dst_offset,
void usbd_frame_zero(struct usb_page_cache *cache, usb_frlength_t offset,
usb_frlength_t len);
void usbd_start_re_enumerate(struct usb_device *udev);
+usb_error_t
+ usbd_start_set_config(struct usb_device *, uint8_t);
int usb_fifo_attach(struct usb_device *udev, void *priv_sc,
struct mtx *priv_mtx, struct usb_fifo_methods *pm,