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?
Fixes: 2c8c98158946 ("staging: usbip: let client choose device configuration")
Signed-off-by: Niels Dossche <dossche.niels@xxxxxxxxx>
---
I'm developing this tool as part of my master's dissertation in order to
obtain my master's degree. If you'd like more information about the
details of the tool, please let me know.
drivers/usb/usbip/stub_rx.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c
index 325c22008e53..5dd41e8215e0 100644
--- a/drivers/usb/usbip/stub_rx.c
+++ b/drivers/usb/usbip/stub_rx.c
@@ -138,7 +138,9 @@ static int tweak_set_configuration_cmd(struct urb *urb)
req = (struct usb_ctrlrequest *) urb->setup_packet;
config = le16_to_cpu(req->wValue);
+ usb_lock_device(sdev->udev);
err = usb_set_configuration(sdev->udev, config);
+ usb_unlock_device(sdev->udev);
if (err && err != -ENODEV)
dev_err(&sdev->udev->dev, "can't set config #%d, error %d\n",
config, err);
Looks good to me with the above questions answered.
Reviewed-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
Greg, please pick this patch up.
thanks,
-- Shuah
