From: Ioan-Adrian Ratiu <adi@adirat.com>
To: jikos@kernel.org
Cc: pinglinux@gmail.com, linux-usb@vger.kernel.org,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
joshc@ni.com
Subject: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock
Date: Fri, 20 Nov 2015 22:19:02 +0200 [thread overview]
Message-ID: <1448050742-10777-1-git-send-email-adi@adirat.com> (raw)
In-Reply-To: <1447874755-8673-1-git-send-email-adi@adirat.com>
The critical section protected by usbhid->lock in hid_ctrl() is too
big and because of this it causes a recursive deadlock. "Too big" means
the case statement and the call to hid_input_report() do not need to be
protected by the spinlock (no URB operations are done inside them).
The deadlock happens because in certain rare cases drivers try to grab
the lock while handling the ctrl irq which grabs the lock before them
as described above. For example newer wacom tablets like 056a:033c try
to reschedule proximity reads from wacom_intuos_schedule_prox_event()
calling hid_hw_request() -> usbhid_request() -> usbhid_submit_report()
which tries to grab the usbhid lock already held by hid_ctrl().
There are two ways to get out of this deadlock:
1. Make the drivers work "around" the ctrl critical region, in the
wacom case for ex. by delaying the scheduling of the proximity read
request itself to a workqueue.
2. Shrink the critical region so the usbhid lock protects only the
instructions which modify usbhid state, calling hid_input_report()
with the spinlock unlocked, allowing the device driver to grab the
lock first, finish and then grab the lock afterwards in hid_ctrl().
This patch implements the 2nd solution.
Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
---
drivers/hid/usbhid/hid-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 36712e9..5dd426f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -477,8 +477,6 @@ static void hid_ctrl(struct urb *urb)
struct usbhid_device *usbhid = hid->driver_data;
int unplug = 0, status = urb->status;
- spin_lock(&usbhid->lock);
-
switch (status) {
case 0: /* success */
if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN)
@@ -498,6 +496,8 @@ static void hid_ctrl(struct urb *urb)
hid_warn(urb->dev, "ctrl urb status %d received\n", status);
}
+ spin_lock(&usbhid->lock);
+
if (unplug) {
usbhid->ctrltail = usbhid->ctrlhead;
} else {
--
2.6.3
next prev parent reply other threads:[~2015-11-20 20:19 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-18 19:25 [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu
2015-11-18 20:37 ` Jiri Kosina
2015-11-18 21:05 ` Ioan-Adrian Ratiu
2015-11-18 23:58 ` Josh Cartwright
2015-11-19 6:47 ` Ioan-Adrian Ratiu
2015-11-19 9:10 ` Jiri Kosina
2015-11-19 16:33 ` Ioan-Adrian Ratiu
2015-11-19 21:34 ` Jiri Kosina
2015-11-20 20:08 ` Ioan-Adrian Ratiu
2015-11-19 8:56 ` Jiri Kosina
2015-11-20 20:19 ` Ioan-Adrian Ratiu [this message]
2015-11-29 10:29 ` [PATCH v2] " Ioan-Adrian Ratiu
2016-01-21 1:28 ` Jason Gerecke
2016-01-21 9:36 ` Jiri Kosina
2015-12-01 16:36 ` Jiri Kosina
2015-12-02 0:00 ` Ping Cheng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1448050742-10777-1-git-send-email-adi@adirat.com \
--to=adi@adirat.com \
--cc=jikos@kernel.org \
--cc=joshc@ni.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pinglinux@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).