lkml.org 
[lkml]   [2015]   [Nov]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock
    On Fri, 20 Nov 2015 22:19:02 +0200
    Ioan-Adrian Ratiu <adi@adirat.com> wrote:

    > 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 {

    Hello again

    Can this please be merged in the 4.4? There are quite a few people who have
    their tablets deadlock and don't know how to patch their kernels so are stuck
    waiting for a new release.

    The severity of this issue is much bigger than I initially thought. Since I've
    posted on the wacom mailing list that I have a fix for this deadlock I've
    recived lots of email from people complaining of the same problem on a wide
    range of tablets.

    Some of those people know how to patch a kernel, some found this patch on the
    mailing list and tested it and confirmed that it works on the wacom mailing
    list (you can verify the deadlock + fix on that mailing list).

    Thank you,
    Adrian


    \
     
     \ /
      Last update: 2015-11-29 12:01    [W:2.550 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site