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

[Ioan-Adrian Ratiu <adi@adirat.com>]

On Thu, 19 Nov 2015 10:10:19 +0100 (CET)
Jiri Kosina <jikos@kernel.org> wrote:

> On Thu, 19 Nov 2015, Ioan-Adrian Ratiu wrote:
> 
> > First part of lockdep report:
> > http://imgur.com/clLsCWe
> > 
> > Second part:
> > http://imgur.com/Wa2PzRl
> > 
> > Here are some printk's of mine while reproducing + debugging the issue:
> > http://imgur.com/SETOHT7  
> 
> So the real problem is that Intuos driver is calling hid_hw_request() 
> (which tries to grab the lock in usbhid_submit_report()) while handling 
> the CTRL IRQ (lock gets acquired there).
> 
> So the proper way to fix seems to be delaying the scheduling of the 
> proximity read event in wacom_intuos_inout() to workqueue.
> 
> > I'll continue to research this more in depth, but progress is slow 
> > because I don't have much time, I'm doing this in my spare time because 
> > it's my girlfriend's tablet.  
> 
> Oh, now I understand the level of severity of this bug! :-)
> 
> Thanks,
> 

Yes, exactly, you are beginning to understand! :)  When I've put my 2 variants
above to solve this deadlock, by "removing the call from wacom" at 1) I was
trying to say exactly this, removing it from the irq to a workqueue.

But please understand further my reasoning for submitting this patch. Consider
if this is a bug in the wacom driver or in the usbhid core? IMO
this is a usbhid bug: the critical region in hid_ctrl() is too big, there
is no reason for the call to hid_input_report() to be protected by
usbhid->lock.

The correct way to fix this deadlock is to fix the critical section in
usbhid, not remove the call from the wacom irq. If wacom wants to
reschedule in the irq, it should not deadlock on usbhid. "Fixing" the wacom call
would just work around the critical region bug inside usbhid.

I hope I've made myself clear this time; I really needed to explain this
patch better :( sorry.