Re: [PATCH v3 1/4] Revert "usbip: Implement a match function to fix usbip"

[Shuah Khan <skhan@linuxfoundation.org>]

On 9/22/20 5:07 AM, M. Vefa Bicakci wrote:
> This commit reverts commit 7a2f2974f265 ("usbip: Implement a match
> function to fix usbip").
> 
> In summary, commit d5643d2249b2 ("USB: Fix device driver race")
> inadvertently broke usbip functionality, which I resolved in an incorrect
> manner by introducing a match function to usbip, usbip_match(), that
> unconditionally returns true.
> 
> However, the usbip_match function, as is, causes usbip to take over
> virtual devices used by syzkaller for USB fuzzing, which is a regression
> reported by Andrey Konovalov.
> 
> Furthermore, in conjunction with the fix of another bug, handled by another
> patch titled "usbcore/driver: Fix specific driver selection" in this patch
> set, the usbip_match function causes unexpected USB subsystem behaviour
> when the usbip_host driver is loaded. The unexpected behaviour can be
> qualified as follows:
> - If commit 41160802ab8e ("USB: Simplify USB ID table match") is included
>    in the kernel, then all USB devices are bound to the usbip_host
>    driver, which appears to the user as if all USB devices were
>    disconnected.
> - If the same commit (41160802ab8e) is not in the kernel (as is the case
>    with v5.8.10) then all USB devices are re-probed and re-bound to their
>    original device drivers, which appears to the user as a disconnection
>    and re-connection of USB devices.
> 
> Please note that this commit will make usbip non-operational again,
> until yet another patch in this patch set is merged, titled
> "usbcore/driver: Accommodate usbip".
> 
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Link: https://lore.kernel.org/linux-usb/CAAeHK+zOrHnxjRFs=OE8T=O9208B9HP_oo8RZpyVOZ9AJ54pAA@mail.gmail.com/
> Cc: <stable@vger.kernel.org> # 5.8: 41160802ab8e: USB: Simplify USB ID table match
> 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>
> 
> ---
> v3: New patch in the patch set.
> 
>      Note for stable tree maintainers: I have marked the following commit
>      as a dependency of this patch, because that commit resolves a bug that
>      the next commit in this patch set uncovers, where if a driver does
>      not have an id_table, then its match function is not considered for
>      execution at all.
>        commit 41160802ab8e ("USB: Simplify USB ID table match")
> ---
>   drivers/usb/usbip/stub_dev.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/usb/usbip/stub_dev.c b/drivers/usb/usbip/stub_dev.c
> index 9d7d642022d1..2305d425e6c9 100644
> --- a/drivers/usb/usbip/stub_dev.c
> +++ b/drivers/usb/usbip/stub_dev.c
> @@ -461,11 +461,6 @@ static void stub_disconnect(struct usb_device *udev)
>   	return;
>   }
>   
> -static bool usbip_match(struct usb_device *udev)
> -{
> -	return true;
> -}
> -
>   #ifdef CONFIG_PM
>   
>   /* These functions need usb_port_suspend and usb_port_resume,
> @@ -491,7 +486,6 @@ struct usb_device_driver stub_driver = {
>   	.name		= "usbip-host",
>   	.probe		= stub_probe,
>   	.disconnect	= stub_disconnect,
> -	.match		= usbip_match,
>   #ifdef CONFIG_PM
>   	.suspend	= stub_suspend,
>   	.resume		= stub_resume,
> 

Thank you for finding a solution that works for usbip

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah