Re: [PATCH 3/3] usbip: Make the driver's match function specific

[Shuah Khan <skhan@linuxfoundation.org>]

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.

> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Fixes: 7a2f2974f2 ("usbip: Implement a match function to fix usbip")
> Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/
> Cc: <stable@vger.kernel.org> # 5.8
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Valentina Manea <valentina.manea.m@gmail.com>
> Cc: Shuah Khan <shuah@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: <syzkaller@googlegroups.com>
> Signed-off-by: M. Vefa Bicakci <m.v.b@runbox.com>
> ---
>   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.

thanks,
-- Shuah