Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report
From: Oliver Neukum <hidden>
Date: 2012-04-24 14:26:11
Possibly related (same subject, not in this thread)
- 2012-04-26 · Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report · Greg Kroah-Hartman <gregkh@linuxfoundation.org>
- 2012-04-26 · Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report · Jiri Kosina <hidden>
- 2012-04-25 · Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report · Alan Stern <hidden>
- 2012-04-25 · Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report · Ming Lei <hidden>
- 2012-04-25 · Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report · Oliver Neukum <hidden>
Am Dienstag, 24. April 2012, 06:19:00 schrieb Ming Lei:
quoted hunk ↗ jump to hunk
@@ -486,11 +494,15 @@ static void hid_ctrl(struct urb *urb) if (usbhid->ctrlhead != usbhid->ctrltail && !hid_submit_ctrl(hid)) { /* Successfully submitted next urb in queue */ + if (status != -ECONNRESET) + spin_unlock(&usbhid->unlink_lock); spin_unlock(&usbhid->lock); return; } clear_bit(HID_CTRL_RUNNING, &usbhid->iofl); + if (status != -ECONNRESET) + spin_unlock(&usbhid->unlink_lock); spin_unlock(&usbhid->lock); usb_autopm_put_interface_async(usbhid->intf); wake_up(&usbhid->wait);
Now you race against a double time out CPU A CPU B __usbhid_submit_report() time_after() usb_unlink_urb() -- this has to go to the hardware --> hid_irq_out() if (status != -ECONNRESET) --> no lock hid_submit_out() __usbhid_submit_report() time_after() usb_submit_urb() usb_unlink_urb()
quoted hunk ↗ jump to hunk
@@ -546,8 +558,13 @@ static void __usbhid_submit_report(structhid_device *hid, struct hid_report *re * no race because this is called under * spinlock */ - if (time_after(jiffies, usbhid->last_out + HZ * 5)) + spin_lock(&usbhid->unlink_lock); + if (time_after(jiffies, usbhid->last_out + HZ * 5)) { + spin_unlock(&usbhid->lock); usb_unlink_urb(usbhid->urbout); + spin_lock(&usbhid->lock); + } + spin_unlock(&usbhid->unlink_lock);
AB-BA deadlock Regards Oliver