| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 27 May 2012 09:25:31 +0900
From: Greg KH <gregkh@...uxfoundation.org>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
Cc: torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
alan@...rguk.ukuu.org.uk, Oliver Neukum <oneukum@...e.de>,
Jiri Kosina <jkosina@...e.cz>
Subject: [ 25/91] usbhid: prevent deadlock during timeout
3.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Oliver Neukum <oliver@...kum.org>
commit 8815bb09af21316aeb5f8948b24ac62181670db2 upstream.
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: Oliver Neukum <oneukum@...e.de>
Acked-by: Jiri Kosina <jkosina@...e.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
drivers/hid/usbhid/hid-core.c | 61 +++++++++++++++++++++++++++++++++++++-----
drivers/usb/core/urb.c | 21 ++++++++++++++
include/linux/usb.h | 3 ++
3 files changed, 79 insertions(+), 6 deletions(-)
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -399,6 +399,16 @@ static int hid_submit_ctrl(struct hid_de
* 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(struc
* 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(struc
* 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);
+ }
}
}
--- a/drivers/usb/core/urb.c
+++ b/drivers/usb/core/urb.c
@@ -681,6 +681,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
*
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1379,6 +1379,7 @@ extern int usb_unlink_urb(struct urb *ur
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);
@@ -1391,6 +1392,8 @@ extern struct urb *usb_get_from_anchor(s
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists