Find interface descriptor by bInterfaceNumber

[Youw]

In some cases, the interface number doesn't match its index in the list of interfaces in configuration descriptor; that is the case for some (non USB-compliant) devices, e.g.: - when the device (descriptor) is explicitly non USB-compliant, and its interface numbers simply doesn't match its indexes (e.g. if[3]{0, 3, 4} or if[4]{0,3,1,2}); - other cases when the decice descriptor is broken;

In such cases need to find the descriptor by iteration over all of them, to find the matching bInterfaceNumber. Otherwise we might try to access data outside of the interfaces array, or access incorrect interfaces descriptor which bInterfaceNumber doesn't match the required one.

Related: #1093

[tormodvolden]

For Darwin:

1. I understand the change affects only (and is only needed for) devices where GetPipeProperties fails because of broken endpoint descriptors (workaround introduced in commit aa1d76c). Have you tested this on such a device?
2. The second line of get_endpoints() is: struct darwin_interface *cInterface = &priv->interfaces[iface]; Doesn't it already go wrong here, since iface is used as an index?
3. Also darwin_claim_interface() starts with the same line.

[tormodvolden]

Alright, I guess points 2 and 3 above are fine if the indexing is consistent. Thinking aloud: The cInterface element is initialized in darwin_claim_interface(), first by having .interface set by QueryInterface() and then .num_endpoints and .endpoint_addrs[] filled by get_endpoints().

[tormodvolden]

Can we also describe what problem we are solving, how it would manifest? On a broken device it would show "interface x out of range for device" for a high enough interface number and it would be impossible to use the interface, or potentially try to access the wrong interface for lower interface numbers?

[tormodvolden]

@Eekle Can you please also try this PR, with your device from #1039?

[Youw]

For Darwin:

1. I've explicitly temporary commented out the GetPipeProperties code path to make sure the modified code branch is used, and checked it that way. Checked before and after the fix to make sure it is needed.
2. priv->interfaces is an array of fixed size of MAX_NUM_NTERFACES elements, which is always indexed by the bInterfaceNumber and not the interface index in the descriptor.
3. See above - not an issue.

[Youw]

describe what problem we are solving

Updated PR description. Doesn't look like what you where thinking?

[Eekle]

@Eekle Can you please also try this PR, with your device from #1039?

Could you give me a minimal example I can build to test this? My original bug report was from a downstream program, I'm not familiar with running libusb on its own.

[Eekle]

I have tested v1.0.25 of libusb and this PR, by building and running xusb.exe VID:PID (with the VID and PID of the offending device.

v1.0.25 produces a read access violation. This PR correctly reports the device state:

              total length: 135
         descriptor length: 9
             nb interfaces: 3
              interface[0]: id = 0
interface[0].altsetting[0]: num endpoints = 0
   Class.SubClass.Protocol: 01.01.00
              interface[1]: id = 1
interface[1].altsetting[0]: num endpoints = 0
   Class.SubClass.Protocol: 01.02.00
interface[1].altsetting[1]: num endpoints = 1
   Class.SubClass.Protocol: 01.02.00
       endpoint[0].address: 01
           max packet size: 00C0
          polling interval: 01
              interface[2]: id = 3
interface[2].altsetting[0]: num endpoints = 1
   Class.SubClass.Protocol: 03.00.00
       endpoint[0].address: 82
           max packet size: 0004
          polling interval: 01

[tormodvolden]

@Eekle Thanks a lot! Can you also test current master (without this PR), if it is not too much trouble? Current master won't crash like v1.0.25 but we may see interfaces being mixed up or missing.

[Eekle]

Current master produces the same output:

              total length: 135
         descriptor length: 9
             nb interfaces: 3
              interface[0]: id = 0
interface[0].altsetting[0]: num endpoints = 0
   Class.SubClass.Protocol: 01.01.00
              interface[1]: id = 1
interface[1].altsetting[0]: num endpoints = 0
   Class.SubClass.Protocol: 01.02.00
interface[1].altsetting[1]: num endpoints = 1
   Class.SubClass.Protocol: 01.02.00
       endpoint[0].address: 01
           max packet size: 00C0
          polling interval: 01
              interface[2]: id = 3
interface[2].altsetting[0]: num endpoints = 1
   Class.SubClass.Protocol: 03.00.00
       endpoint[0].address: 82
           max packet size: 0004
          polling interval: 01

[tormodvolden]

Thanks! OK, I guess your device is not broken enough to validate this PR :) Or it is xusb that doesn't access the interfaces much. Shouldn't it at least claim all the interfaces? Can you please attach the full log with debug enabled?

[Youw]

Anything I can do to help progress with this?

[mcuee]

@tormodvolden I think this is safe to merge. Please check if you are okay to merge it or not. Thanks.

[tormodvolden]

Thanks @Youw for fixing this, and @Eekle for your testing.