Re: USB driver ID matching broken

["M. Vefa Bicakci" <m.v.b@runbox.com>]

On 16/09/2020 17.39, Bastien Nocera wrote:
> On Wed, 2020-09-16 at 16:15 +0200, Greg Kroah-Hartman wrote:
>> On Wed, Sep 16, 2020 at 03:33:25PM +0200, Andrey Konovalov wrote:
>>> Hi Bastien, Greg, Alan,
>>>
>>> Looks like commit adb6e6ac20ee ("USB: Also match device drivers
>>> using
>>> the ->match vfunc") broke the USB driver ID matching process. This,
>>> in
>>> turn, led to a complete breakage of the USB fuzzing instance.
>>>
>>> This is how an attempt to connect a USB device looks now:
>>>
>>> [   39.781642][   T12] usb 1-1: new high-speed USB device number 2
>>> using dummy_hcd
>>> [   40.299955][   T12] usb 1-1: New USB device found,
>>> idVendor=0cf3,
>>> idProduct=9271, bcdDevice= 1.08
>>> [   40.303072][   T12] usb 1-1: New USB device strings: Mfr=1,
>>> Product=2, SerialNumber=3
>>> [   40.305678][   T12] usb 1-1: Product: syz
>>> [   40.307041][   T12] usb 1-1: Manufacturer: syz
>>> [   40.308556][   T12] usb 1-1: SerialNumber: syz
>>> [   40.314825][   T12] usbip-host 1-1: 1-1 is not in match_busid
>>> table... skip!
>>> [   42.500114][   T51] usb 1-1: USB disconnect, device number 2
>>>
>>> It seems that when going through the list of registered IDs the
>>> code
>>> tries to match against USB/IP and succeeds as usbip_match() always
>>> returns true.
>>>
>>> I'm not sure what's the best fix for this is.
>>
>> I thought that is what the patch from Bastien was supposed to fix?
>>
>> If it didn't, we can revert it.
> 
> It wasn't. Are you thinking of "usbip: Implement a match function to
> fix usbip" by M. Vefa Bicakci (CC:ed)?
> 
> Seems to me that usbip wants to match *every* device. Wouldn't that be
> a bug in usbip?

Hello all,

I agree with Bastien; it looks like the match function that I had prepared
for the "USB-IP no longer works starting with v5.7.y" bug at [1] is not
appropriate due to the fact that the match function always returns true.

My understanding of how USB-IP works is that the user-space provides the
USB bus identifier of the device to be published via USB-IP to the kernel
via /sys/bus/usb/drivers/usbip-host/match_busid. Given that the bus
identifiers written to match_busid are stored in a table, perhaps this
table can be queried in the usbip_match function to avoid the issue
reported by Andrey while preserving USB-IP's functionality?

If needed, I can prepare a patch implementing this proposal, perhaps after
commit 7a2f2974 ("usbip: Implement a match function to fix usbip") is
reverted. The only catch is that my bandwidth is a bit limited, hence it
may take some time for me to publish a patch.

Sorry for this unexpected bug,

Vefa

[1] https://bugzilla.kernel.org/show_bug.cgi?id=208267