Support malformed interface descriptors (#79)

* Add an invalid interface descriptor to one of the test devices.
* Add a test for getting that interface description.
* Handle the malformed interface/alternate setting descriptors, where
interface/alt numbers don't follow the spec (not 0-based, not contiguous
etc.)

Addresses issue #65.
This commit is contained in:
Sebastian Zagrodzki
2020-04-28 11:32:10 +02:00
committed by GitHub
parent 18f4c1d8a7
commit 9ad54830f4
5 changed files with 49 additions and 18 deletions

View File

@@ -50,14 +50,26 @@ func (c ConfigDesc) String() string {
} }
func (c ConfigDesc) intfDesc(num, alt int) (*InterfaceSetting, error) { func (c ConfigDesc) intfDesc(num, alt int) (*InterfaceSetting, error) {
if num < 0 || num >= len(c.Interfaces) { // In an ideal world, interfaces in the descriptor would be numbered
return nil, fmt.Errorf("interface %d not found, available interfaces 0..%d", num, len(c.Interfaces)-1) // contiguously starting from 0, as required by the specification. In the
// real world however the specification is sometimes ignored:
// https://github.com/google/gousb/issues/65
ifs := make([]int, len(c.Interfaces))
for i := range c.Interfaces {
ifs[i] = c.Interfaces[i].Number
if ifs[i] != num {
continue
}
alts := make([]int, len(c.Interfaces[i].AltSettings))
for a := range c.Interfaces[i].AltSettings {
alts[a] = c.Interfaces[i].AltSettings[a].Alternate
if alts[a] == alt {
return &c.Interfaces[i].AltSettings[a], nil
}
}
return nil, fmt.Errorf("alternate setting %d not found for %s, available alt settings: %v", alt, c.Interfaces[i], alts)
} }
ifInfo := c.Interfaces[num] return nil, fmt.Errorf("interface %d not found, available interface numbers: %v", num, ifs)
if alt < 0 || alt >= len(ifInfo.AltSettings) {
return nil, fmt.Errorf("alternate setting %d not found for %s, available alt settings 0..%d", alt, ifInfo, len(ifInfo.AltSettings)-1)
}
return &ifInfo.AltSettings[alt], nil
} }
// Config represents a USB device set to use a particular configuration. // Config represents a USB device set to use a particular configuration.

View File

@@ -81,6 +81,7 @@ func TestClaimAndRelease(t *testing.T) {
{1, 0, "Fast streaming"}, {1, 0, "Fast streaming"},
{1, 1, "Slower streaming"}, {1, 1, "Slower streaming"},
{1, 2, ""}, {1, 2, ""},
{3, 2, "Interface for https://github.com/google/gousb/issues/65"},
} { } {
if got, err := dev.InterfaceDescription(1, tc.intf, tc.alt); err != nil { if got, err := dev.InterfaceDescription(1, tc.intf, tc.alt); err != nil {
t.Errorf("%s.InterfaceDescription(1, %d, %d): %v", dev, tc.intf, tc.alt, err) t.Errorf("%s.InterfaceDescription(1, %d, %d): %v", dev, tc.intf, tc.alt, err)

View File

@@ -29,7 +29,7 @@ var fakeDevices = []fakeDevice{
devDesc: &DeviceDesc{ devDesc: &DeviceDesc{
Bus: 1, Bus: 1,
Address: 1, Address: 1,
Port: 1, Port: 1,
Spec: Version(2, 0), Spec: Version(2, 0),
Device: Version(1, 0), Device: Version(1, 0),
Vendor: ID(0x9999), Vendor: ID(0x9999),
@@ -73,7 +73,7 @@ var fakeDevices = []fakeDevice{
devDesc: &DeviceDesc{ devDesc: &DeviceDesc{
Bus: 1, Bus: 1,
Address: 2, Address: 2,
Port: 2, Port: 2,
Spec: Version(2, 0), Spec: Version(2, 0),
Device: Version(1, 3), Device: Version(1, 3),
Vendor: ID(0x8888), Vendor: ID(0x8888),
@@ -158,6 +158,18 @@ var fakeDevices = []fakeDevice{
}, },
}, },
}}, }},
}, {
// Malformed descriptior, non contiguous interface
// number (previous interface is #1), alt settings
// not starting from 0.
// See https://github.com/google/gousb/issues/65
Number: 3,
AltSettings: []InterfaceSetting{{
Number: 3,
Alternate: 2,
Class: ClassVendorSpec,
iInterface: 9,
}},
}}, }},
}}, }},
iManufacturer: 1, iManufacturer: 1,
@@ -172,6 +184,7 @@ var fakeDevices = []fakeDevice{
6: "Boring setting", 6: "Boring setting",
7: "Fast streaming", 7: "Fast streaming",
8: "Slower streaming", 8: "Slower streaming",
9: "Interface for https://github.com/google/gousb/issues/65",
}, },
}, },
// Bus 001 Device 003: ID 9999:0002 // Bus 001 Device 003: ID 9999:0002
@@ -181,7 +194,7 @@ var fakeDevices = []fakeDevice{
devDesc: &DeviceDesc{ devDesc: &DeviceDesc{
Bus: 1, Bus: 1,
Address: 3, Address: 3,
Port: 3, Port: 3,
Spec: Version(2, 0), Spec: Version(2, 0),
Device: Version(1, 0), Device: Version(1, 0),
Vendor: ID(0x1111), Vendor: ID(0x1111),

View File

@@ -23,8 +23,7 @@ import (
// InterfaceDesc contains information about a USB interface, extracted from // InterfaceDesc contains information about a USB interface, extracted from
// the descriptor. // the descriptor.
type InterfaceDesc struct { type InterfaceDesc struct {
// Number is the number of this interface, a zero-based index in the array // Number is the number of this interface.
// of interfaces supported by the device configuration.
Number int Number int
// AltSettings is a list of alternate settings supported by the interface. // AltSettings is a list of alternate settings supported by the interface.
AltSettings []InterfaceSetting AltSettings []InterfaceSetting

View File

@@ -271,7 +271,9 @@ func (libusbImpl) getDeviceDesc(d *libusbDevice) (*DeviceDesc, error) {
Cap: int(cfg.bNumInterfaces), Cap: int(cfg.bNumInterfaces),
} }
c.Interfaces = make([]InterfaceDesc, 0, len(ifaces)) c.Interfaces = make([]InterfaceDesc, 0, len(ifaces))
for ifNum, iface := range ifaces { // a map of interface numbers to a set of alternate settings numbers
hasIntf := make(map[int]map[int]bool)
for _, iface := range ifaces {
if iface.num_altsetting == 0 { if iface.num_altsetting == 0 {
continue continue
} }
@@ -283,7 +285,7 @@ func (libusbImpl) getDeviceDesc(d *libusbDevice) (*DeviceDesc, error) {
Cap: int(iface.num_altsetting), Cap: int(iface.num_altsetting),
} }
descs := make([]InterfaceSetting, 0, len(alts)) descs := make([]InterfaceSetting, 0, len(alts))
for altNum, alt := range alts { for _, alt := range alts {
i := InterfaceSetting{ i := InterfaceSetting{
Number: int(alt.bInterfaceNumber), Number: int(alt.bInterfaceNumber),
Alternate: int(alt.bAlternateSetting), Alternate: int(alt.bAlternateSetting),
@@ -292,12 +294,16 @@ func (libusbImpl) getDeviceDesc(d *libusbDevice) (*DeviceDesc, error) {
Protocol: Protocol(alt.bInterfaceProtocol), Protocol: Protocol(alt.bInterfaceProtocol),
iInterface: int(alt.iInterface), iInterface: int(alt.iInterface),
} }
if ifNum != i.Number {
return nil, fmt.Errorf("config %d interface at index %d has number %d, USB standard states they should be identical", c.Number, ifNum, i.Number) if hasIntf[i.Number][i.Alternate] {
log.Printf("Device on bus %d address %d offered a descriptor for config %d with two different entries with the same interface number (%d) and the same alternate setting number (%d). gousb will use only the first one.", dev.Bus, dev.Address, c.Number, i.Number, i.Alternate)
continue
} }
if altNum != i.Alternate { if hasIntf[i.Number] == nil {
return nil, fmt.Errorf("config %d interface %d alternate settings at index %d has number %d, USB standard states they should be identical", c.Number, i.Number, altNum, i.Alternate) hasIntf[i.Number] = make(map[int]bool)
} }
hasIntf[i.Number][i.Alternate] = true
var ends []C.struct_libusb_endpoint_descriptor var ends []C.struct_libusb_endpoint_descriptor
*(*reflect.SliceHeader)(unsafe.Pointer(&ends)) = reflect.SliceHeader{ *(*reflect.SliceHeader)(unsafe.Pointer(&ends)) = reflect.SliceHeader{
Data: uintptr(unsafe.Pointer(alt.endpoint)), Data: uintptr(unsafe.Pointer(alt.endpoint)),