On Fri, Apr 20, 2012 at 12:11 AM, Oliver Neukum <oneukum@xxxxxxx> wrote:
> Am Donnerstag, 19. April 2012, 15:51:04 schrieb Ming Lei:
>> The URB complete handler may be called by usb_unlink_urb directly,
>> so deadlock will be triggered in __usbhid_submit_report since
>> usbhid->lock is to be acquired in ctrl/out URB complete handler
>> but it is hold before calling usb_unlink_urb.
>>
>> This patch avoids the deadlock by releasing the lock before
>> calling usb_unlink_urb.
>>
>> CC: <stable@xxxxxxxxxxxxxxx>
>> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx>
>> ---
>> drivers/hid/usbhid/hid-core.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
>> index aa1c503..b5d07da 100644
>> --- a/drivers/hid/usbhid/hid-core.c
>> +++ b/drivers/hid/usbhid/hid-core.c
>> @@ -543,11 +543,13 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
>> * the queue is known to run
>> * but an earlier request may be stuck
>> * we may need to time out
>> - * no race because this is called under
>> - * spinlock
>> + * release spinlock to avoid deadlock.
>> */
>> - if (time_after(jiffies, usbhid->last_out + HZ * 5))
>> + if (time_after(jiffies, usbhid->last_out + HZ * 5)) {
>> + spin_unlock(&usbhid->lock);
>> usb_unlink_urb(usbhid->urbout);
>> + spin_lock(&usbhid->lock);
>
> The problem indeed exists on some HCDs.
> I am afraid if you drop the lock there you introduce a race whereby
> you might unlink the wrong request.
The complete handler is called just one time per one submit in either
irq path or unlink path. Secondly, usb_unlink_urb itself is race free.
Finally, usb_unlink_urb was always the last function called inside
__usbhid_submit_report.
So I don't see any races can be introduced by the patch.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
