On 17/09/2020 18.21, Shuah Khan wrote:
On 9/17/20 8:41 AM, M. Vefa Bicakci wrote:
Prior to this commit, the USB-IP subsystem's USB device driver match
function used to match all USB devices (by returning true
unconditionally). Unfortunately, this is not correct behaviour and is
likely the root cause of the bug reported by Andrey Konovalov.
USB-IP should only match USB devices that the user-space asked the kernel
to handle via USB-IP, by writing to the match_busid sysfs file, which is
what this commit aims to achieve. This is done by making the match
function check that the passed in USB device was indeed requested by the
user-space to be handled by USB-IP.
I see two patches 2/2 and 3/3 back to back. What is the difference
between 2/2 and 3/3 versions? They look identical. Please include
changes if any from version to version to make it easier for me to
review.
Hello Shuah,
Sorry for the delayed reply, and thank you for your interest! I realize
that I did not add notes to the patch series regarding the changes between
v1 and v2, and I forgot to label the second patch series as v2.
Patches 2/2 and 3/3 are the same, as you have mentioned. I was addressing
Bastien's code review comments for patch 1/2, and I split that patch into
two separate patches, which is why the second patch series had 3 patches
as opposed to 2.
I realize that you are missing the context; here is a link to the thread:
https://lore.kernel.org/linux-usb/359d080c-5cbb-250a-0ebd-aaba5f5c530d@xxxxxxxxxx/T/
I can copy all patches to you as well, if you would be interested.
All this to say, I am sorry about this small mess, and I will rectify this
with patches I publish in the future.
Reported-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
Fixes: 7a2f2974f2 ("usbip: Implement a match function to fix usbip")
Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@xxxxxxxxxxxxxx/
Cc: <stable@xxxxxxxxxxxxxxx> # 5.8
Cc: Bastien Nocera <hadess@xxxxxxxxxx>
Cc: Valentina Manea <valentina.manea.m@xxxxxxxxx>
Cc: Shuah Khan <shuah@xxxxxxxxxx>
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: <syzkaller@xxxxxxxxxxxxxxxx>
Signed-off-by: M. Vefa Bicakci <m.v.b@xxxxxxxxxx>
---
drivers/usb/usbip/stub_dev.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
index 9d7d642022d1..3d9c8ff6762e 100644
--- a/drivers/usb/usbip/stub_dev.c
+++ b/drivers/usb/usbip/stub_dev.c
@@ -463,7 +463,20 @@ static void stub_disconnect(struct usb_device *udev)
static bool usbip_match(struct usb_device *udev)
{
- return true;
+ bool match;
+ struct bus_id_priv *busid_priv;
+ const char *udev_busid = dev_name(&udev->dev);
+
+ busid_priv = get_busid_priv(udev_busid);
+ if (!busid_priv)
+ return false;
+
+ match = (busid_priv->status != STUB_BUSID_REMOV &&
+ busid_priv->status != STUB_BUSID_OTHER);
+
+ put_busid_priv(busid_priv);
+
+ return match;
}
#ifdef CONFIG_PM
Did you happen to run the usbip test on this patch? If not, can you
please run tools/testing/selftests/drivers/usb/usbip/usbip_test.sh
and make sure there are no regressions.
Ah, this is a very good point! I have been testing the patches on Qubes OS,
which uses usbip to forward USB devices between VMs. To be honest, I was not
aware of the self-tests for usbip, and I will run the self-tests prior to
publishing the next version of the patch series.
thanks,
-- Shuah
Thank you!
Vefa
