Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8815bb09 authored by Oliver Neukum's avatar Oliver Neukum Committed by Greg Kroah-Hartman
Browse files

usbhid: prevent deadlock during timeout 



On some HCDs usb_unlink_urb() can directly call the
completion handler. That limits the spinlocks that can
be taken in the handler to locks not held while calling
usb_unlink_urb()
To prevent a race with resubmission, this patch exposes
usbcore's infrastructure for blocking submission, uses it
and so drops the lock without causing a race in usbhid.

Signed-off-by: default avatarOliver Neukum <oneukum@suse.de>
Acked-by: default avatarJiri Kosina <jkosina@suse.cz>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 166cb70e

drivers/hid/usbhid/hid-core.c

+55 −6
Original line number Diff line number Diff line
@@ -399,6 +399,16 @@ static int hid_submit_ctrl(struct hid_device *hid) 
 * Output interrupt completion handler.

 */



static int irq_out_pump_restart(struct hid_device *hid)

{

	struct usbhid_device *usbhid = hid->driver_data;



	if (usbhid->outhead != usbhid->outtail)

		return hid_submit_out(hid);

	else

		return -1;

}



static void hid_irq_out(struct urb *urb)

{

	struct hid_device *hid = urb->context;
@@ -428,7 +438,7 @@ static void hid_irq_out(struct urb *urb) 
	else

		usbhid->outtail = (usbhid->outtail + 1) & (HID_OUTPUT_FIFO_SIZE - 1);



	if (usbhid->outhead != usbhid->outtail && !hid_submit_out(hid)) {

	if (!irq_out_pump_restart(hid)) {

		/* Successfully submitted next urb in queue */

		spin_unlock_irqrestore(&usbhid->lock, flags);

		return;
@@ -443,6 +453,15 @@ static void hid_irq_out(struct urb *urb) 
/*

 * Control pipe completion handler.

 */

static int ctrl_pump_restart(struct hid_device *hid)

{

	struct usbhid_device *usbhid = hid->driver_data;



	if (usbhid->ctrlhead != usbhid->ctrltail)

		return hid_submit_ctrl(hid);

	else

		return -1;

}



static void hid_ctrl(struct urb *urb)

{
@@ -476,7 +495,7 @@ static void hid_ctrl(struct urb *urb) 
	else

		usbhid->ctrltail = (usbhid->ctrltail + 1) & (HID_CONTROL_FIFO_SIZE - 1);



	if (usbhid->ctrlhead != usbhid->ctrltail && !hid_submit_ctrl(hid)) {

	if (!ctrl_pump_restart(hid)) {

		/* Successfully submitted next urb in queue */

		spin_unlock(&usbhid->lock);

		return;
@@ -535,11 +554,27 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re 
			 * the queue is known to run

			 * but an earlier request may be stuck

			 * we may need to time out

			 * no race because this is called under

			 * no race because the URB is blocked under

			 * spinlock

			 */

			if (time_after(jiffies, usbhid->last_out + HZ * 5))

			if (time_after(jiffies, usbhid->last_out + HZ * 5)) {

				usb_block_urb(usbhid->urbout);

				/* drop lock to not deadlock if the callback is called */

				spin_unlock(&usbhid->lock);

				usb_unlink_urb(usbhid->urbout);

				spin_lock(&usbhid->lock);

				usb_unblock_urb(usbhid->urbout);

				/*

				 * if the unlinking has already completed

				 * the pump will have been stopped

				 * it must be restarted now

				 */

				if (!test_bit(HID_OUT_RUNNING, &usbhid->iofl))

					if (!irq_out_pump_restart(hid))

						set_bit(HID_OUT_RUNNING, &usbhid->iofl);





			}

		}

		return;

	}
@@ -583,11 +618,25 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re 
		 * the queue is known to run

		 * but an earlier request may be stuck

		 * we may need to time out

		 * no race because this is called under

		 * no race because the URB is blocked under

		 * spinlock

		 */

		if (time_after(jiffies, usbhid->last_ctrl + HZ * 5))

		if (time_after(jiffies, usbhid->last_ctrl + HZ * 5)) {

			usb_block_urb(usbhid->urbctrl);

			/* drop lock to not deadlock if the callback is called */

			spin_unlock(&usbhid->lock);

			usb_unlink_urb(usbhid->urbctrl);

			spin_lock(&usbhid->lock);

			usb_unblock_urb(usbhid->urbctrl);

			/*

			 * if the unlinking has already completed

			 * the pump will have been stopped

			 * it must be restarted now

			 */

			if (!test_bit(HID_CTRL_RUNNING, &usbhid->iofl))

				if (!ctrl_pump_restart(hid))

					set_bit(HID_CTRL_RUNNING, &usbhid->iofl);

		}

	}

}

drivers/usb/core/urb.c

+21 −0
Original line number Diff line number Diff line
@@ -680,6 +680,27 @@ void usb_unpoison_urb(struct urb *urb) 
}

EXPORT_SYMBOL_GPL(usb_unpoison_urb);



/**

 * usb_block_urb - reliably prevent further use of an URB

 * @urb: pointer to URB to be blocked, may be NULL

 *

 * After the routine has run, attempts to resubmit the URB will fail

 * with error -EPERM.  Thus even if the URB's completion handler always

 * tries to resubmit, it will not succeed and the URB will become idle.

 *

 * The URB must not be deallocated while this routine is running.  In

 * particular, when a driver calls this routine, it must insure that the

 * completion handler cannot deallocate the URB.

 */

void usb_block_urb(struct urb *urb)

{

	if (!urb)

		return;



	atomic_inc(&urb->reject);

}

EXPORT_SYMBOL_GPL(usb_block_urb);



/**

 * usb_kill_anchored_urbs - cancel transfer requests en masse

 * @anchor: anchor the requests are bound to

include/linux/usb.h

+3 −0
Original line number Diff line number Diff line
@@ -1369,6 +1369,7 @@ extern int usb_unlink_urb(struct urb *urb); 
extern void usb_kill_urb(struct urb *urb);

extern void usb_poison_urb(struct urb *urb);

extern void usb_unpoison_urb(struct urb *urb);

extern void usb_block_urb(struct urb *urb);

extern void usb_kill_anchored_urbs(struct usb_anchor *anchor);

extern void usb_poison_anchored_urbs(struct usb_anchor *anchor);

extern void usb_unpoison_anchored_urbs(struct usb_anchor *anchor);
@@ -1381,6 +1382,8 @@ extern struct urb *usb_get_from_anchor(struct usb_anchor *anchor); 
extern void usb_scuttle_anchored_urbs(struct usb_anchor *anchor);

extern int usb_anchor_empty(struct usb_anchor *anchor);



#define usb_unblock_urb	usb_unpoison_urb



/**

 * usb_urb_dir_in - check if an URB describes an IN transfer

 * @urb: URB to be checked