Re: [PATCH] usb: usbip: add missing device lock on tweak configuration cmd

[Shuah Khan <skhan@linuxfoundation.org>]

On 4/12/22 2:01 PM, Niels Dossche wrote:
> On 4/12/22 21:45, Shuah Khan wrote:
>> On 4/12/22 10:50 AM, Niels Dossche wrote:
>>> The function documentation of usb_set_configuration says that its
>>> callers should hold the device lock. This lock is held for all
>>> callsites except tweak_set_configuration_cmd. The code path can be
>>> executed for example when attaching a remote USB device.
>>> The solution is to surround the call by the device lock.
>>>
>>> This bug was found using my experimental own-developed static analysis
>>> tool, which reported the missing lock on v5.17.2. I manually verified
>>> this bug report by doing code review as well. I runtime checked that
>>> the required lock is not held. I compiled and runtime tested this on
>>> x86_64 with a USB mouse. After applying this patch, my analyser no
>>> longer reports this potential bug.
>>>
>>
>> How did you run-time check that the lock isn't held? Also x86_64 with a
>> USB mouse - did you test it over loopback?
> 
> I checked the lock isn't held with lockdep_assert_not_held and also by
> checking the return value of usb_trylock_device to be equal to 1 in my testing.
> I checked my USB mouse over loopback yes, binding the mouse for remote use and
> attaching on loopback.
> 

Great. Thank you for a detailed answer.

> 
>>
>>> Fixes: 2c8c98158946 ("staging: usbip: let client choose device configuration")
>>> Signed-off-by: Niels Dossche <dossche.niels@gmail.com>
>>> ---
>>>

>>
>> Looks good to me with the above questions answered.
>>
>> Reviewed-by: Shuah Khan <skhan@linuxfoundation.org>
>>
>> Greg, please pick this patch up.
>>

thanks,
-- Shuah