Re: [PATCH] hid: usbhid: fix possible deadlock in __usbhid_submit_report

[Ming Lei <ming.lei@canonical.com>]

On Sun, Apr 22, 2012 at 8:50 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Sun, 22 Apr 2012, Ming Lei wrote:
>
>> > Although the kerneldoc doesn't actually say so, it should be safe to
>> > assume that usb_unlink_urb calls the completion routine directly _only_
>> > in cases where the unlink succeeded.  (We could add this to the
>> > kerneldoc.)
>> >
>> > Therefore: If the URB completes with status other than -ECONNRESET then
>> > you can safely take the lock for resubmission.  If the URB completes
>> > with status == -ECONNRESET then you know it was unlinked, so you don't
>> > need to take the lock -- the race has already been lost.
>> >
>> > Does that solve your problem?
>>
>> Not sure if that does work.
>>
>> If the URB completes asynchronously after unlinking, its status is still
>>  -ECONNRESET, so extra race may be caused without holding the lock
>> because complete handler will access some global data.
>
> That would be a completely separate race, right?  So maybe it can use a

Not sure, at least in both usbnet and usbhid cases, the lock is held before
usb_unlink_urb, and the same lock is to be acquired in the URB complete
handler.

> different lock for protection -- and this other lock could be dropped
> before usb_unlink_urb is called.

If the lock which is to be acquired in the URB complete handler is dropped
before calling usb_unlink_urb, one new submitted URB in complete handler
may be unlinked, as mentioned by Oliver already.



Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html