Commit 471e42ad authored by Thomas Pugliese's avatar Thomas Pugliese Committed by Greg Kroah-Hartman
Browse files

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: default avatarThomas Pugliese <thomas.pugliese@gmail.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 6161ae5f

drivers/usb/wusbcore/devconnect.c

+1 −23
Original line number Diff line number Diff line
@@ -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);

}

}



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

drivers/usb/wusbcore/security.c

+57 −41
Original line number Diff line number Diff line
@@ -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;

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



	mutex_unlock(&wusbhc->mutex);

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



	queue_work(wusbd, &wusbhc->gtk_rekey_done_work);

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

	}



	/* 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);

}

drivers/usb/wusbcore/wusbhc.h

+2 −4
Original line number Diff line number Diff line
@@ -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;

};

include/linux/usb/wusb.h

+2 −0
Original line number Diff line number Diff line
@@ -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 {

CRA Git | Maintained and supported by SUSTech CRA and CCSE