lkml.org 
[lkml]   [2016]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.16.y-ckt 180/180] HID: usbhid: fix recursive deadlock
    Date
    3.16.7-ckt24 -stable review patch.  If anyone has any objections, please let me know.

    ---8<------------------------------------------------------------

    From: Ioan-Adrian Ratiu <adi@adirat.com>

    commit e470127e9606b1fa151c4184243e61296d1e0c0f upstream.

    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>
    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
    Cc: Jason Gerecke <killertofu@gmail.com>
    Signed-off-by: Luis Henriques <luis.henriques@canonical.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 b22e5c4e5448..421187ba053a 100644
    --- a/drivers/hid/usbhid/hid-core.c
    +++ b/drivers/hid/usbhid/hid-core.c
    @@ -492,8 +492,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)
    @@ -513,6 +511,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 {
    \
     
     \ /
      Last update: 2016-02-04 00:21    [W:4.105 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site