Re: Bug caused by 53965c79c2db (USB: Fix device driver race)

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

On 20/10/2020 11.28, Alan Stern wrote:
> On Tue, Oct 20, 2020 at 08:03:23AM -0400, M. Vefa Bicakci wrote:
>> On 19/10/2020 13.40, Alan Stern wrote:
>>> On Mon, Oct 19, 2020 at 09:36:00AM +0000, Pany wrote:
[Snipped by Vefa]
>>
>> ... and Pany's system has multiple USB devices manufactured by Apple
>> (including the SD card reader), with the vendor code 0x05ac, which is
>> the value included by the id_table of the apple-mfi-fastcharge driver:
>>
>> Sep 29 15:22:48 fedora.local kernel: usb 2-3: new SuperSpeed Gen 1 USB device number 2 using xhci_hcd
>> Sep 29 15:22:48 fedora.local kernel: usb 2-3: New USB device found, idVendor=05ac, idProduct=8406, bcdDevice= 8.20
>> Sep 29 15:22:48 fedora.local kernel: usb 2-3: New USB device strings: Mfr=3, Product=4, SerialNumber=5
>> Sep 29 15:22:48 fedora.local kernel: usb 2-3: Product: Card Reader
>> Sep 29 15:22:48 fedora.local kernel: usb 2-3: Manufacturer: Apple
>> ...
>> Sep 29 15:22:48 fedora.local kernel: usb 1-5: new full-speed USB device number 3 using xhci_hcd
>> Sep 29 15:22:48 fedora.local kernel: usb 1-5: New USB device found, idVendor=05ac, idProduct=0273, bcdDevice= 6.22
>> Sep 29 15:22:48 fedora.local kernel: usb 1-5: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>> Sep 29 15:22:48 fedora.local kernel: usb 1-5: Product: Apple Internal Keyboard / Trackpad
>> Sep 29 15:22:48 fedora.local kernel: usb 1-5: Manufacturer: Apple Inc.
>> ...
>>
>> One way to fix this issue would be to implement a match function in the
>> apple-mfi-fastcharge driver, possibly instead of an id_table, so that it
>> does not match devices that it cannot bind to. This may require other
>> changes in the USB core that I cannot fathom at the moment.
> 
> How about matching on the vendor ID and the product ID?  That would be
> an easy addition to the ID table.  Do the fastcharge devices have a well
> known product ID?
> 
> Alan Stern

Hello Alan,

Thank you for the feedback! Judging from the driver's code, it looks like
the product identifiers are known, but there unfortunately appear to be
256 possible product identifiers (i.e., 0x00->0xFF):


  23 /* The product ID is defined as starting with 0x12nn, as per the
  24  * "Choosing an Apple Device USB Configuration" section in
  25  * release R9 (2012) of the "MFi Accessory Hardware Specification"
  26  *
  27  * To distinguish an Apple device, a USB host can check the device
  28  * descriptor of attached USB devices for the following fields:
  29  * - Vendor ID: 0x05AC
  30  * - Product ID: 0x12nn
  31  *
  32  * Those checks will be done in .match() and .probe().
  33  */
...
166 static int mfi_fc_probe(struct usb_device *udev)
167 {
...
171
172         idProduct = le16_to_cpu(udev->descriptor.idProduct);
173         /* See comment above mfi_fc_id_table[] */
174         if (idProduct < 0x1200 || idProduct > 0x12ff) {
175                 return -ENODEV;
176         }

A quick look at drivers/usb/core/driver.c::usb_match_device indicates
that it is not yet possible to specify ranges in ID tables of USB
drivers, so I think that we would need to replace the ID table with
a match function. Interestingly enough, the comment block quoted
above mentions the use of a match function as well.

Thank you,

Vefa

As a reminder to myself, removing the ID table from the apple-mfi-fastcharge
driver and implementing a match function in this driver will likely require
cherry-picking the following commit to 5.8.y:
	0ed9498f9ecf ("USB: Simplify USB ID table match")