Re: [PATCH] Devices that ignore USB spec generate invalid modaliases

[Nathaniel McCallum <nathaniel@natemccallum.com>]

On 11/12/2009 06:42 PM, Greg KH wrote:
> On Wed, Nov 11, 2009 at 03:20:23AM -0500, Nathaniel McCallum wrote:
>> Please CC me as I'm not subscribed to LKML.
>>
>> The current code to generate usb modaliases from usb_device_id assumes
>> that the device's bcdDevice descriptor will actually be in BCD format.
>> While this should be a sane assumption, some devices don't follow spec
>> and just use plain old hex.  This causes drivers for these devices to
>> generate invalid modalias lines which will never actually match for the
>> hardware.
>>
>> The following patch adds hex support for bcdDevice in file2alias.c.
>> Drivers for devices which have bcdDevice conforming to BCD will have no
>> change in modalias output.  Drivers for devices which don't conform
>> (primarily usb-storage and ibmcam in my initial survey) should now
>> generate valid modaliases.
>>
>> EXAMPLE OUTPUT (ibmcam; space added to highlight change)
>> Old: usb:v0545p800D d030[10-9] dc*dsc*dp*ic*isc*ip*
>> New: usb:v0545p800D d030a      dc*dsc*dp*ic*isc*ip*
>
> Huh?  The old one had '[]' in it?
>
> What does the bcdDevice for this really look like in the device itself?
> If it is messed up in the descriptor, then how can we know to fix it up
> here?

The device contains 0x030A (an invalid value since 0x0309 + 1 == 0x0310 
in BCD format) in the bcdDevice descriptor.  The driver matches against 
this descriptor (see the usb_device_id table in 
drivers/media/video/usbvideo/ibmcam.c).

There are three possible solutions:
1. Fix the ibmcam driver not to match against bcdDevice.  I'm not sure 
if this is possible.  Nor does it solve the problem for future devices 
which may be misprogrammed.
2. Change *all* usb modaliases to be generated assuming hex ranges. 
This makes file2alias.c a little simpler at the cost of changing a large 
number of existing modaliases.  The impact of this change is potentially 
large.
3. My solution, which is to detect if either bcdDevice_lo or 
bcdDevice_hi is in hexadecimal rather than BCD format.  We do this by 
checking each character to see if it is above 0x9.  If either of these 
fields is in hex format, we switch into hex mode and render the modalias 
appropriately.  This also has some potential impact on userspace tools, 
however, since currently the only modalias that is affected is ibmcam, 
the change is measurably small in impact.

The second bug (related here: http://lkml.org/lkml/2009/11/11/245) is 
just simply a bug in the modalias generation code.  Namely, 0x59 + 1 == 
0x60 in BCD format, not 0x5A.  Therefore, you cannot rely on i++ to 
increment. Fixing it has no impact other than fixing a broken modalias 
(which occurs in usb-storage).

Nathaniel McCallum