usb: wusbcore: fix deadlock in wusbhc_gtk_rekey

When multiple wireless USB devices are connected and one of the devices
disconnects, the host will distribute a new group key to the remaining
devicese using wusbhc_gtk_rekey.  wusbhc_gtk_rekey takes the
wusbhc->mutex and holds it while it submits a URB to set the new key.
This causes a deadlock in wa_urb_enqueue when it calls a device lookup
helper function that takes the same lock.

This patch changes wusbhc_gtk_rekey to submit a work item to set the GTK
so that the URB is submitted without holding wusbhc->mutex.

Signed-off-by: Thomas Pugliese <thomas.pugliese@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/wusbcore/devconnect.c b/drivers/usb/wusbcore/devconnect.c
index 5107ca999d4cba87a1b1ca899d1949204d71b31c..f14e7929ba2278dae601788ab4f07ae657e2e2bb 100644 (file)
--- a/drivers/usb/wusbcore/devconnect.c
+++ b/drivers/usb/wusbcore/devconnect.c
@@ -97,18 +97,12 @@ static void wusbhc_devconnect_acked_work(struct work_struct *work);
 
 static void wusb_dev_free(struct wusb_dev *wusb_dev)
 {
-       if (wusb_dev) {
-               kfree(wusb_dev->set_gtk_req);
-               usb_free_urb(wusb_dev->set_gtk_urb);
-               kfree(wusb_dev);
-       }
+       kfree(wusb_dev);
 }
 
 static struct wusb_dev *wusb_dev_alloc(struct wusbhc *wusbhc)
 {
        struct wusb_dev *wusb_dev;
-       struct urb *urb;
-       struct usb_ctrlrequest *req;
 
        wusb_dev = kzalloc(sizeof(*wusb_dev), GFP_KERNEL);
        if (wusb_dev == NULL)
@@ -118,22 +112,6 @@ static struct wusb_dev *wusb_dev_alloc(struct wusbhc *wusbhc)
 
        INIT_WORK(&wusb_dev->devconnect_acked_work, wusbhc_devconnect_acked_work);
 
-       urb = usb_alloc_urb(0, GFP_KERNEL);
-       if (urb == NULL)
-               goto err;
-       wusb_dev->set_gtk_urb = urb;
-
-       req = kmalloc(sizeof(*req), GFP_KERNEL);
-       if (req == NULL)
-               goto err;
-       wusb_dev->set_gtk_req = req;
-
-       req->bRequestType = USB_DIR_OUT | USB_TYPE_STANDARD | USB_RECIP_DEVICE;
-       req->bRequest = USB_REQ_SET_DESCRIPTOR;
-       req->wValue = cpu_to_le16(USB_DT_KEY << 8 | wusbhc->gtk_index);
-       req->wIndex = 0;
-       req->wLength = cpu_to_le16(wusbhc->gtk.descr.bLength);
-
        return wusb_dev;
 err:
        wusb_dev_free(wusb_dev);

diff --git a/drivers/usb/wusbcore/security.c b/drivers/usb/wusbcore/security.c
index dd88441c8f7891e0d242e1107d6ff585a30999e4..4c40d0dbf53d45c35ae5b05798f1f09566ffc29e 100644 (file)
--- a/drivers/usb/wusbcore/security.c
+++ b/drivers/usb/wusbcore/security.c
@@ -29,19 +29,16 @@
 #include <linux/export.h>
 #include "wusbhc.h"
 
-static void wusbhc_set_gtk_callback(struct urb *urb);
-static void wusbhc_gtk_rekey_done_work(struct work_struct *work);
+static void wusbhc_gtk_rekey_work(struct work_struct *work);
 
 int wusbhc_sec_create(struct wusbhc *wusbhc)
 {
        wusbhc->gtk.descr.bLength = sizeof(wusbhc->gtk.descr) + sizeof(wusbhc->gtk.data);
        wusbhc->gtk.descr.bDescriptorType = USB_DT_KEY;
        wusbhc->gtk.descr.bReserved = 0;
+       wusbhc->gtk_index = 0;
 
-       wusbhc->gtk_index = wusb_key_index(0, WUSB_KEY_INDEX_TYPE_GTK,
-                                          WUSB_KEY_INDEX_ORIGINATOR_HOST);
-
-       INIT_WORK(&wusbhc->gtk_rekey_done_work, wusbhc_gtk_rekey_done_work);
+       INIT_WORK(&wusbhc->gtk_rekey_work, wusbhc_gtk_rekey_work);
 
        return 0;
 }
@@ -113,7 +110,7 @@ int wusbhc_sec_start(struct wusbhc *wusbhc)
        wusbhc_generate_gtk(wusbhc);
 
        result = wusbhc->set_gtk(wusbhc, wusbhc->gtk_tkid,
-                                &wusbhc->gtk.descr.bKeyData, key_size);
+                               &wusbhc->gtk.descr.bKeyData, key_size);
        if (result < 0)
                dev_err(wusbhc->dev, "cannot set GTK for the host: %d\n",
                        result);
@@ -129,7 +126,7 @@ int wusbhc_sec_start(struct wusbhc *wusbhc)
  */
 void wusbhc_sec_stop(struct wusbhc *wusbhc)
 {
-       cancel_work_sync(&wusbhc->gtk_rekey_done_work);
+       cancel_work_sync(&wusbhc->gtk_rekey_work);
 }
 
 
@@ -185,12 +182,14 @@ static int wusb_dev_set_encryption(struct usb_device *usb_dev, int value)
 static int wusb_dev_set_gtk(struct wusbhc *wusbhc, struct wusb_dev *wusb_dev)
 {
        struct usb_device *usb_dev = wusb_dev->usb_dev;
+       u8 key_index = wusb_key_index(wusbhc->gtk_index,
+               WUSB_KEY_INDEX_TYPE_GTK, WUSB_KEY_INDEX_ORIGINATOR_HOST);
 
        return usb_control_msg(
                usb_dev, usb_sndctrlpipe(usb_dev, 0),
                USB_REQ_SET_DESCRIPTOR,
                USB_DIR_OUT | USB_TYPE_STANDARD | USB_RECIP_DEVICE,
-               USB_DT_KEY << 8 | wusbhc->gtk_index, 0,
+               USB_DT_KEY << 8 | key_index, 0,
                &wusbhc->gtk.descr, wusbhc->gtk.descr.bLength,
                1000);
 }
@@ -520,24 +519,55 @@ error_kzalloc:
  * Once all connected and authenticated devices have received the new
  * GTK, switch the host to using it.
  */
-static void wusbhc_gtk_rekey_done_work(struct work_struct *work)
+static void wusbhc_gtk_rekey_work(struct work_struct *work)
 {
-       struct wusbhc *wusbhc = container_of(work, struct wusbhc, gtk_rekey_done_work);
+       struct wusbhc *wusbhc = container_of(work,
+                                       struct wusbhc, gtk_rekey_work);
        size_t key_size = sizeof(wusbhc->gtk.data);
+       int port_idx;
+       struct wusb_dev *wusb_dev, *wusb_dev_next;
+       LIST_HEAD(rekey_list);
 
        mutex_lock(&wusbhc->mutex);
+       /* generate the new key */
+       wusbhc_generate_gtk(wusbhc);
+       /* roll the gtk index. */
+       wusbhc->gtk_index = (wusbhc->gtk_index + 1) % (WUSB_KEY_INDEX_MAX + 1);
+       /*
+        * Save all connected devices on a list while holding wusbhc->mutex and
+        * take a reference to each one.  Then submit the set key request to
+        * them after releasing the lock in order to avoid a deadlock.
+        */
+       for (port_idx = 0; port_idx < wusbhc->ports_max; port_idx++) {
+               wusb_dev = wusbhc->port[port_idx].wusb_dev;
+               if (!wusb_dev || !wusb_dev->usb_dev
+                       || !wusb_dev->usb_dev->authenticated)
+                       continue;
 
-       if (--wusbhc->pending_set_gtks == 0)
-               wusbhc->set_gtk(wusbhc, wusbhc->gtk_tkid, &wusbhc->gtk.descr.bKeyData, key_size);
-
+               wusb_dev_get(wusb_dev);
+               list_add_tail(&wusb_dev->rekey_node, &rekey_list);
+       }
        mutex_unlock(&wusbhc->mutex);
-}
 
-static void wusbhc_set_gtk_callback(struct urb *urb)
-{
-       struct wusbhc *wusbhc = urb->context;
+       /* Submit the rekey requests without holding wusbhc->mutex. */
+       list_for_each_entry_safe(wusb_dev, wusb_dev_next, &rekey_list,
+               rekey_node) {
+               list_del_init(&wusb_dev->rekey_node);
+               dev_dbg(&wusb_dev->usb_dev->dev, "%s: rekey device at port %d\n",
+                       __func__, wusb_dev->port_idx);
+
+               if (wusb_dev_set_gtk(wusbhc, wusb_dev) < 0) {
+                       dev_err(&wusb_dev->usb_dev->dev, "%s: rekey device at port %d failed\n",
+                               __func__, wusb_dev->port_idx);
+               }
+               wusb_dev_put(wusb_dev);
+       }
 
-       queue_work(wusbd, &wusbhc->gtk_rekey_done_work);
+       /* Switch the host controller to use the new GTK. */
+       mutex_lock(&wusbhc->mutex);
+       wusbhc->set_gtk(wusbhc, wusbhc->gtk_tkid,
+               &wusbhc->gtk.descr.bKeyData, key_size);
+       mutex_unlock(&wusbhc->mutex);
 }
 
 /**
@@ -553,26 +583,12 @@ static void wusbhc_set_gtk_callback(struct urb *urb)
  */
 void wusbhc_gtk_rekey(struct wusbhc *wusbhc)
 {
-       static const size_t key_size = sizeof(wusbhc->gtk.data);
-       int p;
-
-       wusbhc_generate_gtk(wusbhc);
-
-       for (p = 0; p < wusbhc->ports_max; p++) {
-               struct wusb_dev *wusb_dev;
-
-               wusb_dev = wusbhc->port[p].wusb_dev;
-               if (!wusb_dev || !wusb_dev->usb_dev || !wusb_dev->usb_dev->authenticated)
-                       continue;
-
-               usb_fill_control_urb(wusb_dev->set_gtk_urb, wusb_dev->usb_dev,
-                                    usb_sndctrlpipe(wusb_dev->usb_dev, 0),
-                                    (void *)wusb_dev->set_gtk_req,
-                                    &wusbhc->gtk.descr, wusbhc->gtk.descr.bLength,
-                                    wusbhc_set_gtk_callback, wusbhc);
-               if (usb_submit_urb(wusb_dev->set_gtk_urb, GFP_KERNEL) == 0)
-                       wusbhc->pending_set_gtks++;
-       }
-       if (wusbhc->pending_set_gtks == 0)
-               wusbhc->set_gtk(wusbhc, wusbhc->gtk_tkid, &wusbhc->gtk.descr.bKeyData, key_size);
+       /*
+        * We need to submit a URB to the downstream WUSB devices in order to
+        * change the group key.  This can't be done while holding the
+        * wusbhc->mutex since that is also taken in the urb_enqueue routine
+        * and will cause a deadlock.  Instead, queue a work item to do
+        * it when the lock is not held
+        */
+       queue_work(wusbd, &wusbhc->gtk_rekey_work);
 }

diff --git a/drivers/usb/wusbcore/wusbhc.h b/drivers/usb/wusbcore/wusbhc.h
index 711b1952b114ab09322a2b11289cf7aa5fe0d433..6bd3b819a6b56b2c37f552ba5177f71df80cd626 100644 (file)
--- a/drivers/usb/wusbcore/wusbhc.h
+++ b/drivers/usb/wusbcore/wusbhc.h
@@ -97,6 +97,7 @@ struct wusb_dev {
        struct kref refcnt;
        struct wusbhc *wusbhc;
        struct list_head cack_node;     /* Connect-Ack list */
+       struct list_head rekey_node;    /* GTK rekey list */
        u8 port_idx;
        u8 addr;
        u8 beacon_type:4;
@@ -107,8 +108,6 @@ struct wusb_dev {
        struct usb_wireless_cap_descriptor *wusb_cap_descr;
        struct uwb_mas_bm availability;
        struct work_struct devconnect_acked_work;
-       struct urb *set_gtk_urb;
-       struct usb_ctrlrequest *set_gtk_req;
        struct usb_device *usb_dev;
 };
 
@@ -296,8 +295,7 @@ struct wusbhc {
        } __attribute__((packed)) gtk;
        u8 gtk_index;
        u32 gtk_tkid;
-       struct work_struct gtk_rekey_done_work;
-       int pending_set_gtks;
+       struct work_struct gtk_rekey_work;
 
        struct usb_encryption_descriptor *ccm1_etd;
 };

diff --git a/include/linux/usb/wusb.h b/include/linux/usb/wusb.h
index 0c4d4ca370ec9ba2e88c90b5d9a34b1efc9bc557..eeb28329fa3c9a17087834e2bb2f4d054757a005 100644 (file)
--- a/include/linux/usb/wusb.h
+++ b/include/linux/usb/wusb.h
@@ -271,6 +271,8 @@ static inline u8 wusb_key_index(int index, int type, int originator)
 #define WUSB_KEY_INDEX_TYPE_GTK                        2
 #define WUSB_KEY_INDEX_ORIGINATOR_HOST         0
 #define WUSB_KEY_INDEX_ORIGINATOR_DEVICE       1
+/* bits 0-3 used for the key index. */
+#define WUSB_KEY_INDEX_MAX                     15
 
 /* A CCM Nonce, defined in WUSB1.0[6.4.1] */
 struct aes_ccm_nonce {