Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1279314

Re: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock

Show all headers | View raw

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-18 20:30 +0100
  Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-18 21:40 +0100
    Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-18 22:10 +0100
      Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Josh Cartwright <joshc@ni.com> - 2015-11-19 01:00 +0100
        Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-19 07:50 +0100
          Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 10:20 +0100
            Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-19 17:40 +0100
              Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 22:40 +0100
                Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-20 21:10 +0100
      Re: [PATCH] hid: usbhid: hid-core: fix recursive deadlock Jiri Kosina <jikos@kernel.org> - 2015-11-19 10:00 +0100
  [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-20 21:20 +0100
    Re: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock Ioan-Adrian Ratiu <adi@adirat.com> - 2015-11-29 11:30 +0100

csiph-web