| Message ID | 20200922110703.720960-2-m.v.b@runbox.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d6407613c1e2ef90213dee388aa16b6e1bd08cbc |
| Headers | show |
| Series | Fixes for usbip and specialised USB driver selection | expand |
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
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,
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(-)