Skip to content

Commit

Permalink
descriptor: Replace parse_descriptor() function
Browse files Browse the repository at this point in the history
This function had a few problems:

 - it takes two buffers as parameters but knows nothing about their
length, making it easy to overrun them.

 - callers make unwarranted assumptions about the alignment of
structures that are passed to it (it assumes there's no padding)

 - it has tricky pointer arithmetic and masking

With this new formulation, it's easier to see what's being read/written,
especially the destination. It's now very clear that the destination is
not being overrun because we are simply assigning to struct fields.

Also convert byte swapping macros to inline functions for more type
safety.

References #1460
  • Loading branch information
seanm authored and tormodvolden committed May 28, 2024
1 parent 2a138c6 commit 2c32efa
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 68 deletions.
162 changes: 95 additions & 67 deletions libusb/descriptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,54 +30,18 @@
* for detected devices
*/

#define READ_LE16(p) ((uint16_t) \
(((uint16_t)((p)[1]) << 8) | \
((uint16_t)((p)[0]))))

#define READ_LE32(p) ((uint32_t) \
(((uint32_t)((p)[3]) << 24) | \
((uint32_t)((p)[2]) << 16) | \
((uint32_t)((p)[1]) << 8) | \
((uint32_t)((p)[0]))))

static void parse_descriptor(const void *source, const char *descriptor, void *dest)
static inline uint16_t ReadLittleEndian16(const uint8_t p[2])
{
const uint8_t *sp = source;
uint8_t *dp = dest;
char field_type;

while (*descriptor) {
field_type = *descriptor++;
switch (field_type) {
case 'b': /* 8-bit byte */
*dp++ = *sp++;
break;
case 'w': /* 16-bit word, convert from little endian to CPU */
dp += ((uintptr_t)dp & 1); /* Align to 16-bit word boundary */

*((uint16_t *)dp) = READ_LE16(sp);
sp += 2;
dp += 2;
break;
case 'd': /* 32-bit word, convert from little endian to CPU (4-byte align dst before write). */
dp += 4 - ((uintptr_t)dp & 3); /* Align to 32-bit word boundary */
return (uint16_t)p[1] << 8 |
(uint16_t)p[0];
}

*((uint32_t *)dp) = READ_LE32(sp);
sp += 4;
dp += 4;
break;
case 'i': /* 32-bit word, convert from little endian to CPU (no dst alignment before write) */
*((uint32_t *)dp) = READ_LE32(sp);
sp += 4;
dp += 4;
break;
case 'u': /* 16 byte UUID */
memcpy(dp, sp, 16);
sp += 16;
dp += 16;
break;
}
}
static inline uint32_t ReadLittleEndian32(const uint8_t p[4])
{
return (uint32_t)p[3] << 24 |
(uint32_t)p[2] << 16 |
(uint32_t)p[1] << 8 |
(uint32_t)p[0];
}

static void clear_endpoint(struct libusb_endpoint_descriptor *endpoint)
Expand Down Expand Up @@ -114,10 +78,16 @@ static int parse_endpoint(struct libusb_context *ctx,
return parsed;
}

if (header->bLength >= LIBUSB_DT_ENDPOINT_AUDIO_SIZE)
parse_descriptor(buffer, "bbbbwbbb", endpoint);
else
parse_descriptor(buffer, "bbbbwb", endpoint);
endpoint->bLength = buffer[0];
endpoint->bDescriptorType = buffer[1];
endpoint->bEndpointAddress = buffer[2];
endpoint->bmAttributes = buffer[3];
endpoint->wMaxPacketSize = ReadLittleEndian16(&buffer[4]);
endpoint->bInterval = buffer[6];
if (header->bLength >= LIBUSB_DT_ENDPOINT_AUDIO_SIZE) {
endpoint->bRefresh = buffer[7];
endpoint->bSynchAddress = buffer[8];
}

buffer += header->bLength;
size -= header->bLength;
Expand Down Expand Up @@ -217,7 +187,15 @@ static int parse_interface(libusb_context *ctx,
usb_interface->altsetting = altsetting;

ifp = altsetting + usb_interface->num_altsetting;
parse_descriptor(buffer, "bbbbbbbbb", ifp);
ifp->bLength = buffer[0];
ifp->bDescriptorType = buffer[1];
ifp->bInterfaceNumber = buffer[2];
ifp->bAlternateSetting = buffer[3];
ifp->bNumEndpoints = buffer[4];
ifp->bInterfaceClass = buffer[5];
ifp->bInterfaceSubClass = buffer[6];
ifp->bInterfaceProtocol = buffer[7];
ifp->iInterface = buffer[8];
if (ifp->bDescriptorType != LIBUSB_DT_INTERFACE) {
usbi_err(ctx, "unexpected descriptor 0x%x (expected 0x%x)",
ifp->bDescriptorType, LIBUSB_DT_INTERFACE);
Expand Down Expand Up @@ -363,7 +341,14 @@ static int parse_configuration(struct libusb_context *ctx,
return LIBUSB_ERROR_IO;
}

parse_descriptor(buffer, "bbwbbbbb", config);
config->bLength = buffer[0];
config->bDescriptorType = buffer[1];
config->wTotalLength = ReadLittleEndian16(&buffer[2]);
config->bNumInterfaces = buffer[4];
config->bConfigurationValue = buffer[5];
config->iConfiguration = buffer[6];
config->bmAttributes = buffer[7];
config->MaxPower = buffer[8];
if (config->bDescriptorType != LIBUSB_DT_CONFIG) {
usbi_err(ctx, "unexpected descriptor 0x%x (expected 0x%x)",
config->bDescriptorType, LIBUSB_DT_CONFIG);
Expand Down Expand Up @@ -742,7 +727,11 @@ int API_EXPORTED libusb_get_ss_endpoint_companion_descriptor(
*ep_comp = malloc(sizeof(**ep_comp));
if (!*ep_comp)
return LIBUSB_ERROR_NO_MEM;
parse_descriptor(buffer, "bbbbw", *ep_comp);
(*ep_comp)->bLength = buffer[0];
(*ep_comp)->bDescriptorType = buffer[1];
(*ep_comp)->bMaxBurst = buffer[2];
(*ep_comp)->bmAttributes = buffer[3];
(*ep_comp)->wBytesPerInterval = ReadLittleEndian16(&buffer[4]);
return LIBUSB_SUCCESS;
}
return LIBUSB_ERROR_NOT_FOUND;
Expand Down Expand Up @@ -795,7 +784,10 @@ static int parse_bos(struct libusb_context *ctx,
if (!_bos)
return LIBUSB_ERROR_NO_MEM;

parse_descriptor(buffer, "bbwb", _bos);
_bos->bLength = buffer[0];
_bos->bDescriptorType = buffer[1];
_bos->wTotalLength = ReadLittleEndian16(&buffer[2]);
_bos->bNumDeviceCaps = buffer[4];
buffer += _bos->bLength;
size -= _bos->bLength;

Expand Down Expand Up @@ -945,7 +937,10 @@ int API_EXPORTED libusb_get_usb_2_0_extension_descriptor(
if (!_usb_2_0_extension)
return LIBUSB_ERROR_NO_MEM;

parse_descriptor(dev_cap, "bbbd", _usb_2_0_extension);
_usb_2_0_extension->bLength = dev_cap->bLength;
_usb_2_0_extension->bDescriptorType = dev_cap->bDescriptorType;
_usb_2_0_extension->bDevCapabilityType = dev_cap->bDevCapabilityType;
_usb_2_0_extension->bmAttributes = ReadLittleEndian32(dev_cap->dev_capability_data);

*usb_2_0_extension = _usb_2_0_extension;
return LIBUSB_SUCCESS;
Expand Down Expand Up @@ -1000,7 +995,14 @@ int API_EXPORTED libusb_get_ss_usb_device_capability_descriptor(
if (!_ss_usb_device_cap)
return LIBUSB_ERROR_NO_MEM;

parse_descriptor(dev_cap, "bbbbwbbw", _ss_usb_device_cap);
_ss_usb_device_cap->bLength = dev_cap->bLength;
_ss_usb_device_cap->bDescriptorType = dev_cap->bDescriptorType;
_ss_usb_device_cap->bDevCapabilityType = dev_cap->bDevCapabilityType;
_ss_usb_device_cap->bmAttributes = dev_cap->dev_capability_data[0];
_ss_usb_device_cap->wSpeedSupported = ReadLittleEndian16(&dev_cap->dev_capability_data[1]);
_ss_usb_device_cap->bFunctionalitySupport = dev_cap->dev_capability_data[3];
_ss_usb_device_cap->bU1DevExitLat = dev_cap->dev_capability_data[4];
_ss_usb_device_cap->bU2DevExitLat = ReadLittleEndian16(&dev_cap->dev_capability_data[5]);

*ss_usb_device_cap = _ss_usb_device_cap;
return LIBUSB_SUCCESS;
Expand Down Expand Up @@ -1041,8 +1043,14 @@ int API_EXPORTED libusb_get_ssplus_usb_device_capability_descriptor(
return LIBUSB_ERROR_IO;
}

/* We can only parse the non-variable size part of the SuperSpeedPlus descriptor. The attributes have to be read "manually". */
parse_descriptor(dev_cap, "bbbbiww", &parsedDescriptor);
const uint8_t* dev_capability_data = dev_cap->dev_capability_data;
parsedDescriptor.bLength = dev_cap->bLength;
parsedDescriptor.bDescriptorType = dev_cap->bDescriptorType;
parsedDescriptor.bDevCapabilityType = dev_cap->bDevCapabilityType;
parsedDescriptor.bReserved = dev_capability_data[0];
parsedDescriptor.bmAttributes = ReadLittleEndian32(&dev_capability_data[1]);
parsedDescriptor.wFunctionalitySupport = ReadLittleEndian16(&dev_capability_data[5]);
parsedDescriptor.wReserved = ReadLittleEndian16(&dev_capability_data[7]);

uint8_t numSublikSpeedAttributes = (parsedDescriptor.bmAttributes & 0xF) + 1;
_ssplus_cap = malloc(sizeof(struct libusb_ssplus_usb_device_capability_descriptor) + numSublikSpeedAttributes * sizeof(struct libusb_ssplus_sublink_attribute));
Expand All @@ -1067,7 +1075,7 @@ int API_EXPORTED libusb_get_ssplus_usb_device_capability_descriptor(
/* Read the attributes */
uint8_t* base = ((uint8_t*)dev_cap) + LIBUSB_BT_SSPLUS_USB_DEVICE_CAPABILITY_SIZE;
for(uint8_t i = 0 ; i < _ssplus_cap->numSublinkSpeedAttributes ; i++) {
uint32_t attr = READ_LE32(base + i * sizeof(uint32_t));
uint32_t attr = ReadLittleEndian32(base + i * sizeof(uint32_t));
_ssplus_cap->sublinkSpeedAttributes[i].ssid = attr & 0x0f;
_ssplus_cap->sublinkSpeedAttributes[i].mantissa = attr >> 16;
_ssplus_cap->sublinkSpeedAttributes[i].exponent = (attr >> 4) & 0x3 ;
Expand Down Expand Up @@ -1137,7 +1145,11 @@ int API_EXPORTED libusb_get_container_id_descriptor(libusb_context *ctx,
if (!_container_id)
return LIBUSB_ERROR_NO_MEM;

parse_descriptor(dev_cap, "bbbbu", _container_id);
_container_id->bLength = dev_cap->bLength;
_container_id->bDescriptorType = dev_cap->bDescriptorType;
_container_id->bDevCapabilityType = dev_cap->bDevCapabilityType;
_container_id->bReserved = dev_cap->dev_capability_data[0];
memcpy(_container_id->ContainerID, &dev_cap->dev_capability_data[1], 16);

*container_id = _container_id;
return LIBUSB_SUCCESS;
Expand Down Expand Up @@ -1193,7 +1205,11 @@ int API_EXPORTED libusb_get_platform_descriptor(libusb_context *ctx,
if (!_platform_descriptor)
return LIBUSB_ERROR_NO_MEM;

parse_descriptor(dev_cap, "bbbbu", _platform_descriptor);
_platform_descriptor->bLength = dev_cap->bLength;
_platform_descriptor->bDescriptorType = dev_cap->bDescriptorType;
_platform_descriptor->bDevCapabilityType = dev_cap->bDevCapabilityType;
_platform_descriptor->bReserved = dev_cap->dev_capability_data[0];
memcpy(_platform_descriptor->PlatformCapabilityUUID, &(dev_cap->dev_capability_data[1]), 16);

/* Capability data is located after reserved byte and 128-bit UUID */
uint8_t* capability_data = dev_cap->dev_capability_data + 1 + 16;
Expand Down Expand Up @@ -1314,7 +1330,8 @@ static int parse_iad_array(struct libusb_context *ctx,
/* First pass: Iterate through desc list, count number of IADs */
iad_array->length = 0;
while (consumed < size) {
parse_descriptor(buf, "bb", &header);
header.bLength = buf[0];
header.bDescriptorType = buf[1];
if (header.bLength < 2) {
usbi_err(ctx, "invalid descriptor bLength %d",
header.bLength);
Expand All @@ -1338,11 +1355,22 @@ static int parse_iad_array(struct libusb_context *ctx,
consumed = 0;
i = 0;
while (consumed < size) {
parse_descriptor(buffer, "bb", &header);
if (header.bDescriptorType == LIBUSB_DT_INTERFACE_ASSOCIATION)
parse_descriptor(buffer, "bbbbbbbb", &iad[i++]);
buffer += header.bLength;
consumed += header.bLength;
header.bLength = buffer[0];
header.bDescriptorType = buffer[1];
if (header.bDescriptorType == LIBUSB_DT_INTERFACE_ASSOCIATION) {
iad[i].bLength = buffer[0];
iad[i].bDescriptorType = buffer[1];
iad[i].bFirstInterface = buffer[2];
iad[i].bInterfaceCount = buffer[3];
iad[i].bFunctionClass = buffer[4];
iad[i].bFunctionSubClass = buffer[5];
iad[i].bFunctionProtocol = buffer[6];
iad[i].iFunction = buffer[7];
i++;
}

buffer += header.bLength;
consumed += header.bLength;
}
}

Expand Down
2 changes: 1 addition & 1 deletion libusb/version_nano.h
Original file line number Diff line number Diff line change
@@ -1 +1 @@
#define LIBUSB_NANO 11906
#define LIBUSB_NANO 11907

0 comments on commit 2c32efa

Please sign in to comment.