From 76b72c6b0e12e963f1bbf88d7d838ed5c93df10a Mon Sep 17 00:00:00 2001 From: Stephen Date: Tue, 25 Mar 2025 12:16:38 -0700 Subject: [PATCH 1/9] Properly determine libusb read size for large reports Fixes #274 --- libusb/hid.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/libusb/hid.c b/libusb/hid.c index d2ceef5d3..7b7109daf 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -95,6 +95,8 @@ struct hid_device_ { int interface; uint16_t report_descriptor_size; + /* Includes report number. */ + size_t max_input_report_size; /* Endpoint information */ int input_endpoint; @@ -135,6 +137,7 @@ static libusb_context *usb_context = NULL; uint16_t get_usb_code_for_current_locale(void); static int return_data(hid_device *dev, unsigned char *data, size_t length); +static int hid_get_report_descriptor_libusb(libusb_device_handle *handle, int interface_num, uint16_t expected_report_descriptor_size, unsigned char *buf, size_t buf_size); static hid_device *new_hid_device(void) { @@ -276,6 +279,74 @@ static int get_usage(uint8_t *report_descriptor, size_t size, return -1; /* failure */ } +/* Retrieves the largest input report size (in bytes) from the report descriptor. + + Requires an opened device with *claimed interface*. + + The return value is the size on success and -1 on failure. */ +static size_t get_max_input_report_size(libusb_device_handle *handle, int interface_num, uint16_t expected_report_descriptor_size) +{ + int i = 0; + int size_code; + int data_len, key_size; + + int64_t report_size = 0, report_count = 0; + ssize_t max_size = -1; + + unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; + + int desc_size = hid_get_report_descriptor_libusb(handle, interface_num, expected_report_descriptor_size, report_descriptor, sizeof(report_descriptor)); + if (desc_size < 0) { + return -1; + } + + while (i < desc_size) { + int key = report_descriptor[i]; + int key_cmd = key & 0xfc; + + if ((key & 0xf0) == 0xf0) { + /* This is a Long Item. The next byte contains the + length of the data section (value) for this key. + See the HID specification, version 1.11, section + 6.2.2.3, titled "Long Items." */ + if (i+1 < desc_size) + data_len = report_descriptor[i+1]; + else + data_len = 0; /* malformed report */ + key_size = 3; + } else { + /* This is a Short Item. The bottom two bits of the + key contain the size code for the data section + (value) for this key. Refer to the HID + specification, version 1.11, section 6.2.2.2, + titled "Short Items." */ + size_code = key & 0x3; + data_len = (size_code < 3) ? size_code : 4; + key_size = 1; + } + + if (key_cmd == 0x94) { + report_count = get_bytes(report_descriptor, desc_size, data_len, i); + } + if (key_cmd == 0x74) { + report_size = get_bytes(report_descriptor, desc_size, data_len, i); + } + if (key_cmd == 0x80) { // Input + /* report_size is in bits. Determine the total size (count * size), + convert to bytes (rounded up), and add one byte for the report + number. */ + ssize_t size = (((report_count * report_size) + 7) / 8) + 1; + if (size > max_size) + max_size = size; + } + + /* Skip over this key and it's associated data */ + i += data_len + key_size; + } + + return max_size; +} + #if defined(__FreeBSD__) && __FreeBSD__ < 10 /* The libusb version included in FreeBSD < 10 doesn't have this function. In mainline libusb, it's inlined in libusb.h. This function will bear a striking @@ -1024,7 +1095,14 @@ static void *read_thread(void *param) int res; hid_device *dev = param; uint8_t *buf; - const size_t length = dev->input_ep_max_packet_size; + size_t length; + if (dev->max_input_report_size > 0) { + length = dev->max_input_report_size; + } else { + /* If we were unable to reliably determine the maximum input size, fall back + to the max packet size. */ + length = dev->input_ep_max_packet_size; + } /* Set up the transfer object. */ buf = (uint8_t*) malloc(length); @@ -1216,6 +1294,7 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa dev->interface = intf_desc->bInterfaceNumber; dev->report_descriptor_size = get_report_descriptor_size_from_interface_descriptors(intf_desc); + dev->max_input_report_size = get_max_input_report_size(dev->device_handle, dev->interface, dev->report_descriptor_size); dev->input_endpoint = 0; dev->input_ep_max_packet_size = 0; From 09017e50bbaa61a1ff1be32527039d648cf83c8d Mon Sep 17 00:00:00 2001 From: Stephen Date: Thu, 27 Mar 2025 12:00:09 -0700 Subject: [PATCH 2/9] Fixes to report size calculation and added tests --- CMakeLists.txt | 4 +- libusb/CMakeLists.txt | 4 + libusb/hid.c | 62 +++++++----- libusb/test/CMakeLists.txt | 69 ++++++++++++++ libusb/test/max_input_report_size_test.c | 115 +++++++++++++++++++++++ 5 files changed, 228 insertions(+), 26 deletions(-) create mode 100644 libusb/test/CMakeLists.txt create mode 100644 libusb/test/max_input_report_size_test.c diff --git a/CMakeLists.txt b/CMakeLists.txt index d7086813c..b47fcd24a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -70,8 +70,8 @@ if(HIDAPI_ENABLE_ASAN) endif() endif() -if(WIN32) - # so far only Windows has tests +if(WIN32 OR HIDAPI_WITH_LIBUSB) + # so far only Windows and LibUSB have tests option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" ${IS_DEBUG_BUILD}) else() set(HIDAPI_WITH_TESTS OFF) diff --git a/libusb/CMakeLists.txt b/libusb/CMakeLists.txt index 4c458c569..c86987021 100644 --- a/libusb/CMakeLists.txt +++ b/libusb/CMakeLists.txt @@ -105,3 +105,7 @@ if(HIDAPI_INSTALL_TARGETS) endif() hidapi_configure_pc("${PROJECT_ROOT}/pc/hidapi-libusb.pc.in") + +if(HIDAPI_WITH_TESTS) + add_subdirectory(test) +endif() diff --git a/libusb/hid.c b/libusb/hid.c index 7b7109daf..68ad17ee1 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -279,27 +279,18 @@ static int get_usage(uint8_t *report_descriptor, size_t size, return -1; /* failure */ } -/* Retrieves the largest input report size (in bytes) from the report descriptor. - - Requires an opened device with *claimed interface*. - +/* Retrieves the largest input report size (in bytes) from the passed in report descriptor. The return value is the size on success and -1 on failure. */ -static size_t get_max_input_report_size(libusb_device_handle *handle, int interface_num, uint16_t expected_report_descriptor_size) +static size_t get_max_input_report_size(uint8_t * report_descriptor, int desc_size) { int i = 0; int size_code; int data_len, key_size; - int64_t report_size = 0, report_count = 0; + int64_t report_size = -1, report_count = -1; + ssize_t cur_size = 0; ssize_t max_size = -1; - unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; - - int desc_size = hid_get_report_descriptor_libusb(handle, interface_num, expected_report_descriptor_size, report_descriptor, sizeof(report_descriptor)); - if (desc_size < 0) { - return -1; - } - while (i < desc_size) { int key = report_descriptor[i]; int key_cmd = key & 0xfc; @@ -325,26 +316,41 @@ static size_t get_max_input_report_size(libusb_device_handle *handle, int interf key_size = 1; } - if (key_cmd == 0x94) { + if (key_cmd == 0x94) { /* Report Count */ report_count = get_bytes(report_descriptor, desc_size, data_len, i); } - if (key_cmd == 0x74) { + if (key_cmd == 0x74) { /* Report Size */ report_size = get_bytes(report_descriptor, desc_size, data_len, i); } - if (key_cmd == 0x80) { // Input - /* report_size is in bits. Determine the total size (count * size), - convert to bytes (rounded up), and add one byte for the report - number. */ - ssize_t size = (((report_count * report_size) + 7) / 8) + 1; - if (size > max_size) - max_size = size; + if (key_cmd == 0x80) { /* Input */ + if (report_count < 0 || report_size < 0) { + /* We are missing size or count. That isn't good. */ + return 0; + } + cur_size += (report_count * report_size); + } + if (key_cmd == 0x84) { /* Report ID */ + if (cur_size > max_size) { + max_size = cur_size; + } + cur_size = 0; } /* Skip over this key and it's associated data */ i += data_len + key_size; } - return max_size; + if (cur_size > max_size) { + max_size = cur_size; + } + + if (max_size < 0) { + return -1; + } + + /* report_size is in bits. Determine the total size convert to bytes + (rounded up), and add one byte for the report number. */ + return ((max_size + 7) / 8) + 1; } #if defined(__FreeBSD__) && __FreeBSD__ < 10 @@ -1294,7 +1300,15 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa dev->interface = intf_desc->bInterfaceNumber; dev->report_descriptor_size = get_report_descriptor_size_from_interface_descriptors(intf_desc); - dev->max_input_report_size = get_max_input_report_size(dev->device_handle, dev->interface, dev->report_descriptor_size); + + unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; + + int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor)); + if (desc_size > 0) { + dev->max_input_report_size = get_max_input_report_size(report_descriptor, desc_size); + } else { + dev->max_input_report_size = -1; + } dev->input_endpoint = 0; dev->input_ep_max_packet_size = 0; diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt new file mode 100644 index 000000000..95d9a2784 --- /dev/null +++ b/libusb/test/CMakeLists.txt @@ -0,0 +1,69 @@ +add_executable(max_input_report_size_test max_input_report_size_test.c) +set_target_properties(max_input_report_size_test + PROPERTIES + C_STANDARD 11 + C_STANDARD_REQUIRED TRUE +) + +if(TARGET usb-1.0) + target_link_libraries(max_input_report_size_test PRIVATE usb-1.0) +else() + include(FindPkgConfig) + pkg_check_modules(libusb REQUIRED IMPORTED_TARGET libusb-1.0>=1.0.9) + target_link_libraries(max_input_report_size_test PRIVATE PkgConfig::libusb) +endif() + +target_link_libraries(max_input_report_size_test PUBLIC hidapi_include) + +# Each test case requires 2 files: +# .pp_data - textual representation of HIDP_PREPARSED_DATA; +# _expected.rpt_desc - reconstructed HID Report Descriptor out of .pp_data file; +# +# (Non-required by test): +# _real.dpt_desc - the original report rescriptor used to create a test case; +set(HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES + 046D_C52F_0001_000C + 046D_C52F_0001_FF00 + 046D_C52F_0002_0001 + 046D_C52F_0002_FF00 + 17CC_1130_0000_FF01 + 046D_0A37_0001_000C + 046A_0011_0006_0001 + 046D_C077_0002_0001 + 046D_C283_0004_0001 + 046D_B010_0006_0001 + 046D_B010_0002_FF00 + 046D_B010_0002_0001 + 046D_B010_0001_FF00 + 046D_B010_0001_000C + 046D_C534_0001_000C + 046D_C534_0001_FF00 + 046D_C534_0002_0001 + 046D_C534_0002_FF00 + 046D_C534_0006_0001 + 046D_C534_0080_0001 + 047F_C056_0001_000C + 047F_C056_0003_FFA0 + 047F_C056_0005_000B + 045E_02FF_0005_0001 + 1532_00A3_0002_0001 +) + +set(CMAKE_VERSION_SUPPORTS_ENVIRONMENT_MODIFICATION "3.22") + +foreach(TEST_CASE ${HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES}) + set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data") + if(NOT EXISTS "${TEST_PP_DATA}") + message(FATAL_ERROR "Missing '${TEST_PP_DATA}' file for '${TEST_CASE}' test case") + endif() + set(TEST_EXPECTED_DESCRIPTOR "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}_expected.rpt_desc") + if(NOT EXISTS "${TEST_EXPECTED_DESCRIPTOR}") + message(FATAL_ERROR "Missing '${TEST_EXPECTED_DESCRIPTOR}' file for '${TEST_CASE}' test case") + endif() + + add_test(NAME "LibUsbHidMaxInputReportSizeTest_${TEST_CASE}" + COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_EXPECTED_DESCRIPTOR}" + WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" + #WORKING_DIRECTORY "$" + ) +endforeach() diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c new file mode 100644 index 000000000..340f8e92f --- /dev/null +++ b/libusb/test/max_input_report_size_test.c @@ -0,0 +1,115 @@ +#include +#include +#include +#include +#include +#include + +#include "../hid.c" + +static ssize_t parse_max_input_report_size(const char * filename) +{ + FILE* file = fopen(filename, "r"); + if (file == NULL) { + fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); + return -1; + } + + char line[256]; + { + while (fgets(line, sizeof(line), file) != NULL) { + unsigned short temp_ushort; + if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %hu\n", &temp_ushort) == 1) { + fclose(file); + return (ssize_t)temp_ushort; + } + } + } + + fprintf(stderr, "Unable to find pp_data->caps_info[0]->ReportByteLength in %s\n", filename); + fclose(file); + + return -1; +} + +static bool read_hex_data_from_text_file(const char *filename, unsigned char *data_out, size_t data_size, size_t *actual_read) +{ + size_t read_index = 0; + FILE* file = fopen(filename, "r"); + if (file == NULL) { + fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); + return false; + } + + bool result = true; + unsigned int val; + char buf[16]; + while (fscanf(file, "%15s", buf) == 1) { + if (sscanf(buf, "0x%X", &val) != 1) { + fprintf(stderr, "Invalid HEX text ('%s') file, got %s\n", filename, buf); + result = false; + goto end; + } + + if (read_index >= data_size) { + fprintf(stderr, "Buffer for file read is too small. Got only %zu bytes to read '%s'\n", data_size, filename); + result = false; + goto end; + } + + if (val > (unsigned char)-1) { + fprintf(stderr, "Invalid HEX text ('%s') file, got a value of: %u\n", filename, val); + result = false; + goto end; + } + + data_out[read_index] = (unsigned char) val; + + read_index++; + } + + if (!feof(file)) { + fprintf(stderr, "Invalid HEX text ('%s') file - failed to read all values\n", filename); + result = false; + goto end; + } + + *actual_read = read_index; + +end: + fclose(file); + return result; +} + + +int main(int argc, char* argv[]) +{ + if (argc != 3) { + fprintf(stderr, "Expected 2 arguments for the test ('<>.pp_data' and '<>_expected.rpt_desc'), got: %d\n", argc - 1); + return EXIT_FAILURE; + } + + printf("Checking: '%s' / '%s'\n", argv[1], argv[2]); + + unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; + size_t report_descriptor_size = 0; + if (!read_hex_data_from_text_file(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) { + return EXIT_FAILURE; + } + + ssize_t expected = parse_max_input_report_size(argv[1]); + if (expected < 0) { + fprintf(stderr, "Unable to expected max input report size from %s\n", argv[1]); + return EXIT_FAILURE; + } + + ssize_t res = (ssize_t)get_max_input_report_size(report_descriptor, report_descriptor_size); + + if (res != expected) { + fprintf(stderr, "Failed to properly compute size. Got %zd, expected %zd\n", res, expected); + return EXIT_FAILURE; + } else { + printf("Properly computed size: %zd\n", res); + return EXIT_SUCCESS; + } +} From 415db44ef3614db9c28d545370ce0ab57af72873 Mon Sep 17 00:00:00 2001 From: Stephen Date: Thu, 27 Mar 2025 12:32:55 -0700 Subject: [PATCH 3/9] Allow computation of max report size for other types --- libusb/hid.c | 31 ++++++++----- libusb/test/max_input_report_size_test.c | 57 +++++++++++++++++------- 2 files changed, 61 insertions(+), 27 deletions(-) diff --git a/libusb/hid.c b/libusb/hid.c index 68ad17ee1..281efef7e 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -69,6 +69,12 @@ extern "C" { #define DETACH_KERNEL_DRIVER #endif +enum report_descr_type { + REPORT_DESCR_INPUT = 0x80, + REPORT_DESCR_OUTPUT = 0x90, + REPORT_DESCR_FEATURE = 0xB0, +}; + /* Uncomment to enable the retrieval of Usage and Usage Page in hid_enumerate(). Warning, on platforms different from FreeBSD this is very invasive as it requires the detach @@ -279,17 +285,17 @@ static int get_usage(uint8_t *report_descriptor, size_t size, return -1; /* failure */ } -/* Retrieves the largest input report size (in bytes) from the passed in report descriptor. +/* Retrieves the largest report size (in bytes) from the passed in report descriptor. The return value is the size on success and -1 on failure. */ -static size_t get_max_input_report_size(uint8_t * report_descriptor, int desc_size) +static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, enum report_descr_type report_type) { int i = 0; int size_code; int data_len, key_size; int64_t report_size = -1, report_count = -1; - ssize_t cur_size = 0; - ssize_t max_size = -1; + size_t cur_size = 0; + size_t max_size = 0; while (i < desc_size) { int key = report_descriptor[i]; @@ -322,7 +328,7 @@ static size_t get_max_input_report_size(uint8_t * report_descriptor, int desc_si if (key_cmd == 0x74) { /* Report Size */ report_size = get_bytes(report_descriptor, desc_size, data_len, i); } - if (key_cmd == 0x80) { /* Input */ + if (key_cmd == report_type) { /* Input / Output / Feature */ if (report_count < 0 || report_size < 0) { /* We are missing size or count. That isn't good. */ return 0; @@ -344,13 +350,14 @@ static size_t get_max_input_report_size(uint8_t * report_descriptor, int desc_si max_size = cur_size; } - if (max_size < 0) { - return -1; + if (max_size == 0) { + // No matching reports found + return 0; + } else { + /* report_size is in bits. Determine the total size convert to bytes + (rounded up), and add one byte for the report number. */ + return ((max_size + 7) / 8) + 1; } - - /* report_size is in bits. Determine the total size convert to bytes - (rounded up), and add one byte for the report number. */ - return ((max_size + 7) / 8) + 1; } #if defined(__FreeBSD__) && __FreeBSD__ < 10 @@ -1305,7 +1312,7 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor)); if (desc_size > 0) { - dev->max_input_report_size = get_max_input_report_size(report_descriptor, desc_size); + dev->max_input_report_size = get_max_report_size(report_descriptor, desc_size, REPORT_DESCR_INPUT); } else { dev->max_input_report_size = -1; } diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c index 340f8e92f..462bf2b92 100644 --- a/libusb/test/max_input_report_size_test.c +++ b/libusb/test/max_input_report_size_test.c @@ -7,7 +7,13 @@ #include "../hid.c" -static ssize_t parse_max_input_report_size(const char * filename) +struct max_report_sizes { + size_t input; + size_t output; + size_t feature; +}; + +static int parse_max_input_report_size(const char * filename, struct max_report_sizes * sizes) { FILE* file = fopen(filename, "r"); if (file == NULL) { @@ -20,16 +26,20 @@ static ssize_t parse_max_input_report_size(const char * filename) while (fgets(line, sizeof(line), file) != NULL) { unsigned short temp_ushort; if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %hu\n", &temp_ushort) == 1) { - fclose(file); - return (ssize_t)temp_ushort; + sizes->input = (size_t)temp_ushort; + } + if (sscanf(line, "pp_data->caps_info[1]->ReportByteLength = %hu\n", &temp_ushort) == 1) { + sizes->output = (size_t)temp_ushort; + } + if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %hu\n", &temp_ushort) == 1) { + sizes->feature = (size_t)temp_ushort; } } } - fprintf(stderr, "Unable to find pp_data->caps_info[0]->ReportByteLength in %s\n", filename); fclose(file); - return -1; + return 0; } static bool read_hex_data_from_text_file(const char *filename, unsigned char *data_out, size_t data_size, size_t *actual_read) @@ -97,19 +107,36 @@ int main(int argc, char* argv[]) return EXIT_FAILURE; } - ssize_t expected = parse_max_input_report_size(argv[1]); - if (expected < 0) { - fprintf(stderr, "Unable to expected max input report size from %s\n", argv[1]); + struct max_report_sizes expected; + if (parse_max_input_report_size(argv[1], &expected) < 0) { + fprintf(stderr, "Unable to get expected max report sizes from %s\n", argv[1]); return EXIT_FAILURE; } - ssize_t res = (ssize_t)get_max_input_report_size(report_descriptor, report_descriptor_size); + struct max_report_sizes computed = { + .input = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_INPUT), + .output = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_OUTPUT), + .feature = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_FEATURE) + }; - if (res != expected) { - fprintf(stderr, "Failed to properly compute size. Got %zd, expected %zd\n", res, expected); - return EXIT_FAILURE; - } else { - printf("Properly computed size: %zd\n", res); - return EXIT_SUCCESS; + int ret = EXIT_SUCCESS; + + if (expected.input != computed.input) { + fprintf(stderr, "Failed to properly compute input size. Got %zu, expected %zu\n", computed.input, expected.input); + ret = EXIT_FAILURE; } + if (expected.output != computed.output) { + fprintf(stderr, "Failed to properly compute output size. Got %zu, expected %zu\n", computed.output, expected.output); + ret = EXIT_FAILURE; + } + if (expected.feature != computed.feature) { + fprintf(stderr, "Failed to properly compute feature size. Got %zu, expected %zu\n", computed.feature, expected.feature); + ret = EXIT_FAILURE; + } + + if (ret == EXIT_SUCCESS) { + printf("Properly computed sizes: %zu, %zu, %zu\n", computed.input, computed.output, computed.feature); + } + + return ret; } From 8b07bcfd4fa737b0c8a9de78ea631927b0b4a0c2 Mon Sep 17 00:00:00 2001 From: Stephen Date: Thu, 27 Mar 2025 12:47:57 -0700 Subject: [PATCH 4/9] Don't assign -1 to size_t. That's silly --- libusb/hid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libusb/hid.c b/libusb/hid.c index 281efef7e..aee5782d9 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -286,7 +286,7 @@ static int get_usage(uint8_t *report_descriptor, size_t size, } /* Retrieves the largest report size (in bytes) from the passed in report descriptor. - The return value is the size on success and -1 on failure. */ + The return value is the size on success and 0 on failure. */ static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, enum report_descr_type report_type) { int i = 0; @@ -1314,7 +1314,7 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa if (desc_size > 0) { dev->max_input_report_size = get_max_report_size(report_descriptor, desc_size, REPORT_DESCR_INPUT); } else { - dev->max_input_report_size = -1; + dev->max_input_report_size = 0; } dev->input_endpoint = 0; From 3b3c70a4898cae545244b68dc457e5648c9d285e Mon Sep 17 00:00:00 2001 From: Stephen Date: Thu, 19 Jun 2025 12:23:31 -0700 Subject: [PATCH 5/9] Do not increase max_report_size by one if not using report numbers --- libusb/hid.c | 7 +++++-- libusb/test/max_input_report_size_test.c | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/libusb/hid.c b/libusb/hid.c index aee5782d9..15e4b8344 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -297,6 +297,8 @@ static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, en size_t cur_size = 0; size_t max_size = 0; + int report_id_used = 0; + while (i < desc_size) { int key = report_descriptor[i]; int key_cmd = key & 0xfc; @@ -336,6 +338,7 @@ static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, en cur_size += (report_count * report_size); } if (key_cmd == 0x84) { /* Report ID */ + report_id_used = 1; if (cur_size > max_size) { max_size = cur_size; } @@ -355,8 +358,8 @@ static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, en return 0; } else { /* report_size is in bits. Determine the total size convert to bytes - (rounded up), and add one byte for the report number. */ - return ((max_size + 7) / 8) + 1; + (rounded up), and add one byte for the report number (if used). */ + return ((max_size + 7) / 8) + report_id_used; } } diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c index 462bf2b92..e7db89333 100644 --- a/libusb/test/max_input_report_size_test.c +++ b/libusb/test/max_input_report_size_test.c @@ -21,6 +21,8 @@ static int parse_max_input_report_size(const char * filename, struct max_report_ return -1; } + int has_report_id = 0; + char line[256]; { while (fgets(line, sizeof(line), file) != NULL) { @@ -34,11 +36,31 @@ static int parse_max_input_report_size(const char * filename, struct max_report_ if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %hu\n", &temp_ushort) == 1) { sizes->feature = (size_t)temp_ushort; } + if (sscanf(line, "pp_data->cap[%*hu]->ReportID = 0x%hu\n", &temp_ushort) == 1) { + if (temp_ushort) { + has_report_id = 1; + } + } } } fclose(file); + // Windows includes ReportID byte in descriptor size even when it is not + // used. Our libusb calculation does not include it, so remove one byte + // from the sizes to make it match. + if (!has_report_id) { + if (sizes->input) { + sizes->input--; + } + if (sizes->output) { + sizes->output--; + } + if (sizes->feature) { + sizes->feature--; + } + } + return 0; } From c358d33b5608dee700b85dd7a5965bf00f3f691b Mon Sep 17 00:00:00 2001 From: Stephen Date: Thu, 19 Jun 2025 12:23:04 -0700 Subject: [PATCH 6/9] Fix compiler warnings --- libusb/hid.c | 2 +- libusb/test/max_input_report_size_test.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libusb/hid.c b/libusb/hid.c index 15e4b8344..0f063896d 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -330,7 +330,7 @@ static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, en if (key_cmd == 0x74) { /* Report Size */ report_size = get_bytes(report_descriptor, desc_size, data_len, i); } - if (key_cmd == report_type) { /* Input / Output / Feature */ + if (key_cmd == (int)report_type) { /* Input / Output / Feature */ if (report_count < 0 || report_size < 0) { /* We are missing size or count. That isn't good. */ return 0; diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c index e7db89333..562f42856 100644 --- a/libusb/test/max_input_report_size_test.c +++ b/libusb/test/max_input_report_size_test.c @@ -36,7 +36,7 @@ static int parse_max_input_report_size(const char * filename, struct max_report_ if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %hu\n", &temp_ushort) == 1) { sizes->feature = (size_t)temp_ushort; } - if (sscanf(line, "pp_data->cap[%*hu]->ReportID = 0x%hu\n", &temp_ushort) == 1) { + if (sscanf(line, "pp_data->cap[%*u]->ReportID = 0x%hu\n", &temp_ushort) == 1) { if (temp_ushort) { has_report_id = 1; } From e19ba07f55d6be0395ecbf204ef793d11cfcd2aa Mon Sep 17 00:00:00 2001 From: Stephen Robinson Date: Tue, 24 Jun 2025 16:32:20 -0700 Subject: [PATCH 7/9] Remove comment Co-authored-by: Ihor Dutchak --- libusb/test/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index 95d9a2784..4df4501e6 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -64,6 +64,5 @@ foreach(TEST_CASE ${HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES}) add_test(NAME "LibUsbHidMaxInputReportSizeTest_${TEST_CASE}" COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_EXPECTED_DESCRIPTOR}" WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" - #WORKING_DIRECTORY "$" ) endforeach() From 39e58ae3b216958f050191d6fb7bc142e0d45b28 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Thu, 30 Jul 2026 17:13:01 +0300 Subject: [PATCH 8/9] Address libusb report-size review feedback Test real report descriptors, distinguish malformed descriptors, cache descriptor data, and run the libusb tests in CI. Assisted-by: codex:gpt-5.6-sol --- .github/workflows/builds.yml | 7 +- libusb/CMakeLists.txt | 1 + libusb/hid.c | 111 ++----- libusb/hidapi_libusb_report_descriptor.h | 153 ++++++++++ libusb/test/CMakeLists.txt | 27 +- libusb/test/max_input_report_size_test.c | 359 +++++++++++++++++------ 6 files changed, 458 insertions(+), 200 deletions(-) create mode 100644 libusb/hidapi_libusb_report_descriptor.h diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 83ebe190a..02c66342e 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -113,14 +113,17 @@ jobs: - name: Configure CMake run: | rm -rf build install - cmake -B build/shared -S hidapisrc -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHIDAPI_ENABLE_ASAN=ON -DCMAKE_INSTALL_PREFIX=install/shared -DHIDAPI_BUILD_HIDTEST=ON "-DCMAKE_C_FLAGS=${GNU_COMPILE_FLAGS}" - cmake -B build/static -S hidapisrc -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHIDAPI_ENABLE_ASAN=ON -DCMAKE_INSTALL_PREFIX=install/static -DBUILD_SHARED_LIBS=FALSE -DHIDAPI_BUILD_HIDTEST=ON "-DCMAKE_C_FLAGS=${GNU_COMPILE_FLAGS}" + cmake -B build/shared -S hidapisrc -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHIDAPI_ENABLE_ASAN=ON -DHIDAPI_WITH_TESTS=ON -DCMAKE_INSTALL_PREFIX=install/shared -DHIDAPI_BUILD_HIDTEST=ON "-DCMAKE_C_FLAGS=${GNU_COMPILE_FLAGS}" + cmake -B build/static -S hidapisrc -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHIDAPI_ENABLE_ASAN=ON -DHIDAPI_WITH_TESTS=ON -DCMAKE_INSTALL_PREFIX=install/static -DBUILD_SHARED_LIBS=FALSE -DHIDAPI_BUILD_HIDTEST=ON "-DCMAKE_C_FLAGS=${GNU_COMPILE_FLAGS}" - name: Build CMake Shared working-directory: build/shared run: make install - name: Build CMake Static working-directory: build/static run: make install + - name: Run CTest + working-directory: build/shared + run: ctest --no-compress-output --output-on-failure - name: Check artifacts uses: andstor/file-existence-action@v2 with: diff --git a/libusb/CMakeLists.txt b/libusb/CMakeLists.txt index 625b61191..a9911d986 100644 --- a/libusb/CMakeLists.txt +++ b/libusb/CMakeLists.txt @@ -2,6 +2,7 @@ list(APPEND HIDAPI_PUBLIC_HEADERS "hidapi_libusb.h") add_library(hidapi_libusb ${HIDAPI_PUBLIC_HEADERS} + hidapi_libusb_report_descriptor.h hid.c ) target_link_libraries(hidapi_libusb PUBLIC hidapi_include) diff --git a/libusb/hid.c b/libusb/hid.c index 05b4efcfa..4ff5726dc 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -51,6 +51,7 @@ #endif #include "hidapi_libusb.h" +#include "hidapi_libusb_report_descriptor.h" #ifndef HIDAPI_THREAD_MODEL_INCLUDE #define HIDAPI_THREAD_MODEL_INCLUDE "hidapi_thread_pthread.h" @@ -71,12 +72,6 @@ extern "C" { #define DETACH_KERNEL_DRIVER #endif -enum report_descr_type { - REPORT_DESCR_INPUT = 0x80, - REPORT_DESCR_OUTPUT = 0x90, - REPORT_DESCR_FEATURE = 0xB0, -}; - /* Uncomment to enable the retrieval of Usage and Usage Page in hid_enumerate(). Warning, on platforms different from FreeBSD this is very invasive as it requires the detach @@ -120,6 +115,8 @@ struct hid_device_ { int interface; uint16_t report_descriptor_size; + uint8_t *report_descriptor; + size_t report_descriptor_length; /* Includes report number. */ size_t max_input_report_size; @@ -195,6 +192,7 @@ static void free_hid_device(hid_device *dev) hid_free_enumeration(dev->device_info); free_hidapi_error(&dev->error); free(dev->last_read_error_str); + free(dev->report_descriptor); /* Free the device itself */ free(dev); @@ -308,84 +306,6 @@ static int get_usage(uint8_t *report_descriptor, size_t size, return -1; /* failure */ } -/* Retrieves the largest report size (in bytes) from the passed in report descriptor. - The return value is the size on success and 0 on failure. */ -static size_t get_max_report_size(uint8_t * report_descriptor, int desc_size, enum report_descr_type report_type) -{ - int i = 0; - int size_code; - int data_len, key_size; - - int64_t report_size = -1, report_count = -1; - size_t cur_size = 0; - size_t max_size = 0; - - int report_id_used = 0; - - while (i < desc_size) { - int key = report_descriptor[i]; - int key_cmd = key & 0xfc; - - if ((key & 0xf0) == 0xf0) { - /* This is a Long Item. The next byte contains the - length of the data section (value) for this key. - See the HID specification, version 1.11, section - 6.2.2.3, titled "Long Items." */ - if (i+1 < desc_size) - data_len = report_descriptor[i+1]; - else - data_len = 0; /* malformed report */ - key_size = 3; - } else { - /* This is a Short Item. The bottom two bits of the - key contain the size code for the data section - (value) for this key. Refer to the HID - specification, version 1.11, section 6.2.2.2, - titled "Short Items." */ - size_code = key & 0x3; - data_len = (size_code < 3) ? size_code : 4; - key_size = 1; - } - - if (key_cmd == 0x94) { /* Report Count */ - report_count = get_bytes(report_descriptor, desc_size, data_len, i); - } - if (key_cmd == 0x74) { /* Report Size */ - report_size = get_bytes(report_descriptor, desc_size, data_len, i); - } - if (key_cmd == (int)report_type) { /* Input / Output / Feature */ - if (report_count < 0 || report_size < 0) { - /* We are missing size or count. That isn't good. */ - return 0; - } - cur_size += (report_count * report_size); - } - if (key_cmd == 0x84) { /* Report ID */ - report_id_used = 1; - if (cur_size > max_size) { - max_size = cur_size; - } - cur_size = 0; - } - - /* Skip over this key and it's associated data */ - i += data_len + key_size; - } - - if (cur_size > max_size) { - max_size = cur_size; - } - - if (max_size == 0) { - // No matching reports found - return 0; - } else { - /* report_size is in bits. Determine the total size convert to bytes - (rounded up), and add one byte for the report number (if used). */ - return ((max_size + 7) / 8) + report_id_used; - } -} - #if defined(__FreeBSD__) && __FreeBSD__ < 10 /* The libusb version included in FreeBSD < 10 doesn't have this function. In mainline libusb, it's inlined in libusb.h. This function will bear a striking @@ -1457,10 +1377,22 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa dev->report_descriptor_size = get_report_descriptor_size_from_interface_descriptors(intf_desc); unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; - int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor)); + if (desc_size > 0) { - dev->max_input_report_size = get_max_report_size(report_descriptor, desc_size, REPORT_DESCR_INPUT); + ssize_t max_input_report_size; + + dev->report_descriptor = (uint8_t *)malloc((size_t)desc_size); + if (dev->report_descriptor) { + memcpy(dev->report_descriptor, report_descriptor, (size_t)desc_size); + dev->report_descriptor_length = (size_t)desc_size; + } + + max_input_report_size = get_max_report_size(report_descriptor, (size_t)desc_size, REPORT_DESCR_INPUT); + if (max_input_report_size > 0) + dev->max_input_report_size = (size_t)max_input_report_size; + else + dev->max_input_report_size = 0; } else { dev->max_input_report_size = 0; } @@ -2158,7 +2090,12 @@ int HID_API_EXPORT_CALL hid_get_report_descriptor(hid_device *dev, unsigned char register_libusb_error(&dev->error, LIBUSB_SUCCESS, NULL); - res = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, buf, buf_size); + if (dev->report_descriptor) { + res = dev->report_descriptor_length < buf_size ? (int)dev->report_descriptor_length : (int)buf_size; + memcpy(buf, dev->report_descriptor, (size_t)res); + } else { + res = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, buf, buf_size); + } if (res < 0) { register_libusb_error(&dev->error, res, "hid_get_report_descriptor"); diff --git a/libusb/hidapi_libusb_report_descriptor.h b/libusb/hidapi_libusb_report_descriptor.h new file mode 100644 index 000000000..d88d5da67 --- /dev/null +++ b/libusb/hidapi_libusb_report_descriptor.h @@ -0,0 +1,153 @@ +#ifndef HIDAPI_LIBUSB_REPORT_DESCRIPTOR_H +#define HIDAPI_LIBUSB_REPORT_DESCRIPTOR_H + +#include +#include +#include + +enum report_descr_type { + REPORT_DESCR_INPUT = 0x80, + REPORT_DESCR_OUTPUT = 0x90, + REPORT_DESCR_FEATURE = 0xB0, +}; + +struct report_global_state { + uint32_t report_size; + uint32_t report_count; + uint8_t report_id; + int report_size_set; + int report_count_set; +}; + +#define REPORT_GLOBAL_STACK_SIZE 16 + +static uint32_t get_report_item_data(const uint8_t *report_descriptor, size_t item_offset, size_t data_len) +{ + uint32_t value = 0; + + for (size_t i = 0; i < data_len; i++) + value |= (uint32_t)report_descriptor[item_offset + 1 + i] << (8 * i); + + return value; +} + +/* Retrieves the largest report size (in bytes) from the passed-in report + descriptor. Returns the size on success, 0 when the descriptor contains no + reports of the requested type, and -1 for a malformed descriptor. */ +static ssize_t get_max_report_size(const uint8_t *report_descriptor, size_t descriptor_size, enum report_descr_type report_type) +{ + struct report_global_state state = {0}; + struct report_global_state state_stack[REPORT_GLOBAL_STACK_SIZE]; + size_t report_bits[256] = {0}; + size_t state_stack_size = 0; + size_t offset = 0; + int report_found = 0; + int report_ids_used = 0; + + if (!report_descriptor && descriptor_size > 0) + return -1; + + while (offset < descriptor_size) { + const uint8_t key = report_descriptor[offset]; + size_t data_len; + size_t key_size; + + if (key == 0xfe) { + /* Long Item: prefix, data size, long-item tag, then data. */ + if (descriptor_size - offset < 3) + return -1; + data_len = report_descriptor[offset + 1]; + key_size = 3; + } else { + const uint8_t size_code = key & 0x3; + data_len = size_code == 3 ? 4 : size_code; + key_size = 1; + } + + if (data_len > descriptor_size - offset - key_size) + return -1; + + if (key != 0xfe) { + const uint8_t key_cmd = key & 0xfc; + const uint32_t value = get_report_item_data(report_descriptor, offset, data_len); + + switch (key_cmd) { + case 0x74: /* Report Size */ + state.report_size = value; + state.report_size_set = 1; + break; + case 0x84: /* Report ID */ + if (data_len != 1 || value == 0) + return -1; + state.report_id = (uint8_t)value; + report_ids_used = 1; + break; + case 0x94: /* Report Count */ + state.report_count = value; + state.report_count_set = 1; + break; + case 0xa4: /* Push */ + if (data_len != 0 || state_stack_size == REPORT_GLOBAL_STACK_SIZE) + return -1; + state_stack[state_stack_size++] = state; + break; + case 0xb4: /* Pop */ + if (data_len != 0 || state_stack_size == 0) + return -1; + state = state_stack[--state_stack_size]; + break; + default: + if (key_cmd == (uint8_t)report_type) { + size_t item_bits; + + if (!state.report_count_set || !state.report_size_set) + return -1; + if (state.report_count != 0 && state.report_size > SIZE_MAX / state.report_count) + return -1; + + item_bits = (size_t)state.report_count * state.report_size; + if (report_bits[state.report_id] > SIZE_MAX - item_bits) + return -1; + + report_bits[state.report_id] += item_bits; + report_found = 1; + } + break; + } + } + + offset += key_size + data_len; + } + + if (state_stack_size != 0) + return -1; + if (!report_found) + return 0; + + if (report_ids_used) { + size_t max_bits = 0; + size_t max_bytes; + + /* Report ID 0 is reserved when Report ID items are used. */ + if (report_bits[0] != 0) + return -1; + + for (size_t report_id = 1; report_id < 256; report_id++) { + if (report_bits[report_id] > max_bits) + max_bits = report_bits[report_id]; + } + + if (max_bits > SIZE_MAX - 7) + return -1; + max_bytes = (max_bits + 7) / 8; + if (max_bytes >= (size_t)PTRDIFF_MAX) + return -1; + return (ssize_t)(max_bytes + 1); + } + + if (report_bits[0] > (size_t)PTRDIFF_MAX - 7) + return -1; + return (ssize_t)((report_bits[0] + 7) / 8); +} + +#endif diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index 4df4501e6..b222c3741 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -5,23 +5,12 @@ set_target_properties(max_input_report_size_test C_STANDARD_REQUIRED TRUE ) -if(TARGET usb-1.0) - target_link_libraries(max_input_report_size_test PRIVATE usb-1.0) -else() - include(FindPkgConfig) - pkg_check_modules(libusb REQUIRED IMPORTED_TARGET libusb-1.0>=1.0.9) - target_link_libraries(max_input_report_size_test PRIVATE PkgConfig::libusb) -endif() - -target_link_libraries(max_input_report_size_test PUBLIC hidapi_include) +target_link_libraries(max_input_report_size_test PRIVATE hidapi_include) # Each test case requires 2 files: # .pp_data - textual representation of HIDP_PREPARSED_DATA; -# _expected.rpt_desc - reconstructed HID Report Descriptor out of .pp_data file; -# -# (Non-required by test): -# _real.dpt_desc - the original report rescriptor used to create a test case; -set(HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES +# _real.rpt_desc - the original report descriptor used to create a test case. +set(HID_REPORT_DESCRIPTOR_TEST_CASES 046D_C52F_0001_000C 046D_C52F_0001_FF00 046D_C52F_0002_0001 @@ -51,18 +40,18 @@ set(HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES set(CMAKE_VERSION_SUPPORTS_ENVIRONMENT_MODIFICATION "3.22") -foreach(TEST_CASE ${HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES}) +foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES}) set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data") if(NOT EXISTS "${TEST_PP_DATA}") message(FATAL_ERROR "Missing '${TEST_PP_DATA}' file for '${TEST_CASE}' test case") endif() - set(TEST_EXPECTED_DESCRIPTOR "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}_expected.rpt_desc") - if(NOT EXISTS "${TEST_EXPECTED_DESCRIPTOR}") - message(FATAL_ERROR "Missing '${TEST_EXPECTED_DESCRIPTOR}' file for '${TEST_CASE}' test case") + set(TEST_REPORT_DESCRIPTOR "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}_real.rpt_desc") + if(NOT EXISTS "${TEST_REPORT_DESCRIPTOR}") + message(FATAL_ERROR "Missing '${TEST_REPORT_DESCRIPTOR}' file for '${TEST_CASE}' test case") endif() add_test(NAME "LibUsbHidMaxInputReportSizeTest_${TEST_CASE}" - COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_EXPECTED_DESCRIPTOR}" + COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_REPORT_DESCRIPTOR}" WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" ) endforeach() diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c index 562f42856..6b95e93bf 100644 --- a/libusb/test/max_input_report_size_test.c +++ b/libusb/test/max_input_report_size_test.c @@ -1,164 +1,339 @@ +#include +#include +#include #include #include #include -#include #include -#include -#include "../hid.c" +#include "hidapi.h" +#include "../hidapi_libusb_report_descriptor.h" struct max_report_sizes { - size_t input; - size_t output; - size_t feature; + size_t input; + size_t output; + size_t feature; }; -static int parse_max_input_report_size(const char * filename, struct max_report_sizes * sizes) +static int parse_expected_report_sizes(const char *filename, struct max_report_sizes *sizes) { - FILE* file = fopen(filename, "r"); - if (file == NULL) { + FILE *file = fopen(filename, "r"); + int found_input = 0; + int found_output = 0; + int found_feature = 0; + int has_report_id = 0; + char line[256]; + + if (!file) { fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); return -1; } - int has_report_id = 0; + while (fgets(line, sizeof(line), file)) { + unsigned int value; - char line[256]; - { - while (fgets(line, sizeof(line), file) != NULL) { - unsigned short temp_ushort; - if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %hu\n", &temp_ushort) == 1) { - sizes->input = (size_t)temp_ushort; - } - if (sscanf(line, "pp_data->caps_info[1]->ReportByteLength = %hu\n", &temp_ushort) == 1) { - sizes->output = (size_t)temp_ushort; - } - if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %hu\n", &temp_ushort) == 1) { - sizes->feature = (size_t)temp_ushort; - } - if (sscanf(line, "pp_data->cap[%*u]->ReportID = 0x%hu\n", &temp_ushort) == 1) { - if (temp_ushort) { - has_report_id = 1; - } - } + if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %u", &value) == 1) { + sizes->input = value; + found_input = 1; + } else if (sscanf(line, "pp_data->caps_info[1]->ReportByteLength = %u", &value) == 1) { + sizes->output = value; + found_output = 1; + } else if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %u", &value) == 1) { + sizes->feature = value; + found_feature = 1; + } else if (sscanf(line, "pp_data->cap[%*u]->ReportID = 0x%x", &value) == 1 && value != 0) { + has_report_id = 1; } } fclose(file); - // Windows includes ReportID byte in descriptor size even when it is not - // used. Our libusb calculation does not include it, so remove one byte - // from the sizes to make it match. - if (!has_report_id) { - if (sizes->input) { + if (!found_input || !found_output || !found_feature) { + fprintf(stderr, "Missing report-size fields in '%s'\n", filename); + return -1; + } + + /* Windows includes a report-ID byte in each ReportByteLength even when + report IDs are not used. The libusb result only includes an actual ID. */ + if (!has_report_id) { + if (sizes->input) sizes->input--; - } - if (sizes->output) { + if (sizes->output) sizes->output--; - } - if (sizes->feature) { + if (sizes->feature) sizes->feature--; - } } return 0; } -static bool read_hex_data_from_text_file(const char *filename, unsigned char *data_out, size_t data_size, size_t *actual_read) +static int append_byte(unsigned char *data, size_t data_size, size_t *data_length, unsigned int value, const char *filename) +{ + if (value > 0xff) { + fprintf(stderr, "Invalid byte value 0x%x in '%s'\n", value, filename); + return -1; + } + if (*data_length >= data_size) { + fprintf(stderr, "Report descriptor in '%s' exceeds %zu bytes\n", filename, data_size); + return -1; + } + + data[(*data_length)++] = (unsigned char)value; + return 0; +} + +static int parse_hid_decode_record(char *line, unsigned char *data, size_t data_size, size_t *data_length, const char *filename) +{ + char *token = strtok(line + 2, " \t\r\n"); + char *end; + unsigned long expected_length; + + if (!token) + return -1; + + expected_length = strtoul(token, &end, 10); + if (*end != '\0' || expected_length > data_size) + return -1; + + while ((token = strtok(NULL, " \t\r\n")) != NULL) { + unsigned long value; + + if (strlen(token) != 2 || !isxdigit((unsigned char)token[0]) || !isxdigit((unsigned char)token[1])) + return -1; + + value = strtoul(token, &end, 16); + if (*end != '\0' || append_byte(data, data_size, data_length, (unsigned int)value, filename) < 0) + return -1; + } + + if (*data_length != expected_length) { + fprintf(stderr, "HID decode record in '%s' declares %lu bytes but contains %zu\n", + filename, expected_length, *data_length); + return -1; + } + + return 0; +} + +static int parse_c_hex_bytes(char *line, unsigned char *data, size_t data_size, size_t *data_length, const char *filename) +{ + char *comment = strstr(line, "//"); + char *cursor = line; + int found = 0; + + if (comment) + *comment = '\0'; + + while ((cursor = strstr(cursor, "0x")) != NULL) { + if (cursor[2] != '\0' && + cursor[3] != '\0' && + isxdigit((unsigned char)cursor[2]) && + isxdigit((unsigned char)cursor[3]) && + !isxdigit((unsigned char)cursor[4])) { + char byte_text[3] = {cursor[2], cursor[3], '\0'}; + unsigned int value = (unsigned int)strtoul(byte_text, NULL, 16); + + if (append_byte(data, data_size, data_length, value, filename) < 0) + return -1; + found = 1; + cursor += 4; + } else { + cursor += 2; + } + } + + return found; +} + +static int parse_trailing_hex_bytes(char *line, unsigned char *data, size_t data_size, size_t *data_length, const char *filename) +{ + char *tokens[128]; + size_t token_count = 0; + size_t first_hex_token; + char *token; + + for (token = strtok(line, " \t\r\n"); token && token_count < 128; token = strtok(NULL, " \t\r\n")) + tokens[token_count++] = token; + + first_hex_token = token_count; + while (first_hex_token > 0) { + const char *candidate = tokens[first_hex_token - 1]; + if (strlen(candidate) != 2 || + !isxdigit((unsigned char)candidate[0]) || + !isxdigit((unsigned char)candidate[1])) + break; + first_hex_token--; + } + + for (size_t i = first_hex_token; i < token_count; i++) { + unsigned int value = (unsigned int)strtoul(tokens[i], NULL, 16); + if (append_byte(data, data_size, data_length, value, filename) < 0) + return -1; + } + + return first_hex_token < token_count; +} + +static bool read_report_descriptor(const char *filename, unsigned char *data, size_t data_size, size_t *data_length) { - size_t read_index = 0; - FILE* file = fopen(filename, "r"); - if (file == NULL) { + char line[HID_API_MAX_REPORT_DESCRIPTOR_SIZE * 4]; + FILE *file = fopen(filename, "r"); + int found_c_hex = 0; + + if (!file) { fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); return false; } + *data_length = 0; - bool result = true; - unsigned int val; - char buf[16]; - while (fscanf(file, "%15s", buf) == 1) { - if (sscanf(buf, "0x%X", &val) != 1) { - fprintf(stderr, "Invalid HEX text ('%s') file, got %s\n", filename, buf); - result = false; - goto end; + /* hid-decode output includes a canonical raw descriptor on its R: line. + Prefer it over the preceding commented disassembly. */ + while (fgets(line, sizeof(line), file)) { + char *cursor = line; + while (isspace((unsigned char)*cursor)) + cursor++; + if (cursor[0] == 'R' && cursor[1] == ':') { + const int result = parse_hid_decode_record(cursor, data, data_size, data_length, filename); + fclose(file); + return result == 0; } + } - if (read_index >= data_size) { - fprintf(stderr, "Buffer for file read is too small. Got only %zu bytes to read '%s'\n", data_size, filename); - result = false; - goto end; - } + /* Several fixtures contain both a raw tool dump and a normalized + usbdescreqparser rendering. Prefer the normalized 0xNN form when it is + present so the same descriptor is not parsed twice. */ + rewind(file); + while (fgets(line, sizeof(line), file)) { + char line_copy[sizeof(line)]; + char *cursor = line; + int result; + + while (isspace((unsigned char)*cursor)) + cursor++; + if (*cursor == '#' || (*cursor == '/' && cursor[1] == '/')) + continue; - if (val > (unsigned char)-1) { - fprintf(stderr, "Invalid HEX text ('%s') file, got a value of: %u\n", filename, val); - result = false; - goto end; + memcpy(line_copy, line, sizeof(line_copy)); + line_copy[sizeof(line_copy) - 1] = '\0'; + result = parse_c_hex_bytes(line_copy, data, data_size, data_length, filename); + if (result < 0) { + fclose(file); + return false; } + if (result > 0) + found_c_hex = 1; + } + if (found_c_hex) { + fclose(file); + return true; + } - data_out[read_index] = (unsigned char) val; + rewind(file); + while (fgets(line, sizeof(line), file)) { + char line_copy[sizeof(line)]; + char *cursor = line; - read_index++; - } + while (isspace((unsigned char)*cursor)) + cursor++; + if (*cursor == '#' || (*cursor == '/' && cursor[1] == '/')) + continue; - if (!feof(file)) { - fprintf(stderr, "Invalid HEX text ('%s') file - failed to read all values\n", filename); - result = false; - goto end; + memcpy(line_copy, line, sizeof(line_copy)); + line_copy[sizeof(line_copy) - 1] = '\0'; + if (parse_trailing_hex_bytes(line_copy, data, data_size, data_length, filename) < 0) { + fclose(file); + return false; + } } - *actual_read = read_index; - -end: fclose(file); - return result; + if (*data_length == 0) { + fprintf(stderr, "No report-descriptor bytes found in '%s'\n", filename); + return false; + } + return true; } +static int test_report_descriptor_parser(void) +{ + static const uint8_t missing_report_size[] = {0x95, 0x01, 0x81, 0x00}; + static const uint8_t truncated_item[] = {0x75}; + static const uint8_t output_only[] = {0x75, 0x08, 0x95, 0x01, 0x91, 0x00}; + static const uint8_t repeated_report_id[] = { + 0x85, 0x01, 0x75, 0x08, 0x95, 0x01, 0x81, 0x00, + 0x85, 0x02, 0x95, 0x01, 0x81, 0x00, + 0x85, 0x01, 0x95, 0x02, 0x81, 0x00, + }; -int main(int argc, char* argv[]) + if (get_max_report_size(missing_report_size, sizeof(missing_report_size), REPORT_DESCR_INPUT) != -1 || + get_max_report_size(truncated_item, sizeof(truncated_item), REPORT_DESCR_INPUT) != -1) { + fprintf(stderr, "Malformed report descriptor was not rejected\n"); + return -1; + } + if (get_max_report_size(output_only, sizeof(output_only), REPORT_DESCR_INPUT) != 0) { + fprintf(stderr, "Missing input report was not reported as zero-sized\n"); + return -1; + } + if (get_max_report_size(repeated_report_id, sizeof(repeated_report_id), REPORT_DESCR_INPUT) != 4) { + fprintf(stderr, "Repeated report ID fields were not accumulated correctly\n"); + return -1; + } + + return 0; +} + +int main(int argc, char *argv[]) { + unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; + size_t report_descriptor_size = 0; + struct max_report_sizes expected = {0}; + struct max_report_sizes computed; + ssize_t input_size; + ssize_t output_size; + ssize_t feature_size; + int ret = EXIT_SUCCESS; + if (argc != 3) { - fprintf(stderr, "Expected 2 arguments for the test ('<>.pp_data' and '<>_expected.rpt_desc'), got: %d\n", argc - 1); + fprintf(stderr, "Expected 2 arguments ('<>.pp_data' and '<>_real.rpt_desc'), got: %d\n", argc - 1); return EXIT_FAILURE; } printf("Checking: '%s' / '%s'\n", argv[1], argv[2]); - unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; - size_t report_descriptor_size = 0; - if (!read_hex_data_from_text_file(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) { + if (test_report_descriptor_parser() < 0) + return EXIT_FAILURE; + if (!read_report_descriptor(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) + return EXIT_FAILURE; + if (parse_expected_report_sizes(argv[1], &expected) < 0) return EXIT_FAILURE; - } - struct max_report_sizes expected; - if (parse_max_input_report_size(argv[1], &expected) < 0) { - fprintf(stderr, "Unable to get expected max report sizes from %s\n", argv[1]); + input_size = get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_INPUT); + output_size = get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_OUTPUT); + feature_size = get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_FEATURE); + if (input_size < 0 || output_size < 0 || feature_size < 0) { + fprintf(stderr, "Failed to parse report descriptor '%s'\n", argv[2]); return EXIT_FAILURE; } - struct max_report_sizes computed = { - .input = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_INPUT), - .output = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_OUTPUT), - .feature = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_FEATURE) - }; - - int ret = EXIT_SUCCESS; + computed.input = (size_t)input_size; + computed.output = (size_t)output_size; + computed.feature = (size_t)feature_size; if (expected.input != computed.input) { - fprintf(stderr, "Failed to properly compute input size. Got %zu, expected %zu\n", computed.input, expected.input); + fprintf(stderr, "Failed to compute input size. Got %zu, expected %zu\n", computed.input, expected.input); ret = EXIT_FAILURE; } if (expected.output != computed.output) { - fprintf(stderr, "Failed to properly compute output size. Got %zu, expected %zu\n", computed.output, expected.output); + fprintf(stderr, "Failed to compute output size. Got %zu, expected %zu\n", computed.output, expected.output); ret = EXIT_FAILURE; } if (expected.feature != computed.feature) { - fprintf(stderr, "Failed to properly compute feature size. Got %zu, expected %zu\n", computed.feature, expected.feature); + fprintf(stderr, "Failed to compute feature size. Got %zu, expected %zu\n", computed.feature, expected.feature); ret = EXIT_FAILURE; } - if (ret == EXIT_SUCCESS) { - printf("Properly computed sizes: %zu, %zu, %zu\n", computed.input, computed.output, computed.feature); - } + if (ret == EXIT_SUCCESS) + printf("Computed report sizes: %zu, %zu, %zu\n", computed.input, computed.output, computed.feature); return ret; } From 8729fcb9ff7bf8aa43940354b4df4c864602cb37 Mon Sep 17 00:00:00 2001 From: Ihor Dutchak Date: Thu, 30 Jul 2026 17:36:12 +0300 Subject: [PATCH 9/9] Harden libusb report size handling Preserve endpoint-packet compatibility, bound descriptor-derived buffers, unwind read-thread initialization failures, avoid non-HID descriptor requests, and expand cross-platform parser coverage. Assisted-by: codex:gpt-5.6-sol Assisted-by: claude-code:claude-fable-5 --- CMakeLists.txt | 10 +-- libusb/hid.c | 97 +++++++++++++++----- libusb/hidapi_libusb_report_descriptor.h | 15 +++- libusb/test/CMakeLists.txt | 4 +- libusb/test/max_input_report_size_test.c | 109 +++++++++++++++++++---- 5 files changed, 186 insertions(+), 49 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0379fe4e3..b128734a3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,9 +38,9 @@ if(APPLE) option(CMAKE_FRAMEWORK "Build macOS/iOS Framework version of the library" OFF) endif() elseif(NOT WIN32) + option(HIDAPI_WITH_LIBUSB "Build LIBUSB-based implementation of HIDAPI" ON) if(CMAKE_SYSTEM_NAME MATCHES "Linux") option(HIDAPI_WITH_HIDRAW "Build HIDRAW-based implementation of HIDAPI" ON) - option(HIDAPI_WITH_LIBUSB "Build LIBUSB-based implementation of HIDAPI" ON) endif() if(CMAKE_SYSTEM_NAME MATCHES "NetBSD") option(HIDAPI_WITH_NETBSD "Build NetBSD/UHID implementation of HIDAPI" ON) @@ -73,14 +73,14 @@ endif() if(WIN32) option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" ${IS_DEBUG_BUILD}) elseif(CMAKE_SYSTEM_NAME MATCHES "Linux" OR APPLE) - # Linux and macOS have virtual-device based tests (uhid / raw-gadget / - # IOHIDUserDevice). Off by default: they need a virtual device at runtime - # and otherwise self-skip. + # Linux and macOS include virtual-device based tests (uhid / raw-gadget / + # IOHIDUserDevice). Keep the full test suite opt-in because those tests need + # a virtual device at runtime and otherwise self-skip. option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" OFF) elseif(HIDAPI_WITH_LIBUSB) option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" ${IS_DEBUG_BUILD}) else() - set(HIDAPI_WITH_TESTS OFF) + option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" OFF) endif() if(HIDAPI_WITH_TESTS) diff --git a/libusb/hid.c b/libusb/hid.c index 0f2535b9a..b19c76440 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -32,6 +32,7 @@ #include #include #include +#include /* Unix */ #include @@ -136,6 +137,7 @@ struct hid_device_ { hidapi_thread_state thread_state; int shutdown_thread; int transfer_loop_finished; + int read_thread_init_error; struct libusb_transfer *transfer; /* List of received input reports. */ @@ -1180,19 +1182,34 @@ static void *read_thread(void *param) { int res; hid_device *dev = (hid_device *) param; - uint8_t *buf; - size_t length; - if (dev->max_input_report_size > 0) { + uint8_t *buf = NULL; + size_t length = (size_t)dev->input_ep_max_packet_size; + + /* Never shrink below the endpoint packet size: some devices pad short + reports to a complete USB packet. */ + if (dev->max_input_report_size > length) length = dev->max_input_report_size; - } else { - /* If we were unable to reliably determine the maximum input size, fall back - to the max packet size. */ - length = dev->input_ep_max_packet_size; + + if (length == 0 || length > INT_MAX) { + LOG("Invalid input report buffer length: %zu\n", length); + dev->read_thread_init_error = LIBUSB_ERROR_INVALID_PARAM; + goto notify_main_thread; } /* Set up the transfer object. */ buf = (uint8_t*) malloc(length); + if (!buf) { + dev->read_thread_init_error = LIBUSB_ERROR_NO_MEM; + goto notify_main_thread; + } + dev->transfer = libusb_alloc_transfer(0); + if (!dev->transfer) { + free(buf); + dev->read_thread_init_error = LIBUSB_ERROR_NO_MEM; + goto notify_main_thread; + } + libusb_fill_interrupt_transfer(dev->transfer, dev->device_handle, dev->input_endpoint, @@ -1207,13 +1224,18 @@ static void *read_thread(void *param) res = libusb_submit_transfer(dev->transfer); if(res < 0) { LOG("libusb_submit_transfer failed: %d %s. Stopping read_thread from running\n", res, libusb_error_name(res)); + dev->read_thread_init_error = res; dev->shutdown_thread = 1; dev->transfer_loop_finished = 1; } +notify_main_thread: /* Notify the main thread that the read thread is up and running. */ hidapi_thread_barrier_wait(&dev->thread_state); + if (dev->read_thread_init_error) + return NULL; + /* Handle all the events. */ while (!dev->shutdown_thread) { res = libusb_handle_events(usb_context); @@ -1328,6 +1350,17 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa int i =0; int res = 0; struct libusb_device_descriptor desc; + + /* hid_open_path() can retry another alternate setting after initialization + fails. Clear all state owned by the previous attempt before doing so. */ + free(dev->report_descriptor); + dev->report_descriptor = NULL; + dev->report_descriptor_length = 0; + dev->max_input_report_size = 0; + dev->shutdown_thread = 0; + dev->transfer_loop_finished = 0; + dev->read_thread_init_error = 0; + libusb_get_device_descriptor(libusb_get_device(dev->device_handle), &desc); #ifdef DETACH_KERNEL_DRIVER @@ -1381,25 +1414,23 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa dev->report_descriptor_size = get_report_descriptor_size_from_interface_descriptors(intf_desc); - unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; - int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor)); + if (intf_desc->bInterfaceClass == LIBUSB_CLASS_HID) { + unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; + int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor)); - if (desc_size > 0) { - ssize_t max_input_report_size; + if (desc_size > 0) { + ssize_t max_input_report_size; - dev->report_descriptor = (uint8_t *)malloc((size_t)desc_size); - if (dev->report_descriptor) { - memcpy(dev->report_descriptor, report_descriptor, (size_t)desc_size); - dev->report_descriptor_length = (size_t)desc_size; - } + dev->report_descriptor = (uint8_t *)malloc((size_t)desc_size); + if (dev->report_descriptor) { + memcpy(dev->report_descriptor, report_descriptor, (size_t)desc_size); + dev->report_descriptor_length = (size_t)desc_size; + } - max_input_report_size = get_max_report_size(report_descriptor, (size_t)desc_size, REPORT_DESCR_INPUT); - if (max_input_report_size > 0) - dev->max_input_report_size = (size_t)max_input_report_size; - else - dev->max_input_report_size = 0; - } else { - dev->max_input_report_size = 0; + max_input_report_size = get_max_report_size(report_descriptor, (size_t)desc_size, REPORT_DESCR_INPUT); + if (max_input_report_size > 0) + dev->max_input_report_size = (size_t)max_input_report_size; + } } dev->input_endpoint = 0; @@ -1442,6 +1473,26 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa /* Wait here for the read thread to be initialized. */ hidapi_thread_barrier_wait(&dev->thread_state); + if (dev->read_thread_init_error) { + hidapi_thread_join(&dev->thread_state); + if (dev->transfer) { + free(dev->transfer->buffer); + dev->transfer->buffer = NULL; + libusb_free_transfer(dev->transfer); + dev->transfer = NULL; + } + + libusb_release_interface(dev->device_handle, dev->interface); +#ifdef DETACH_KERNEL_DRIVER + if (dev->is_driver_detached) { + res = libusb_attach_kernel_driver(dev->device_handle, dev->interface); + if (res < 0) + LOG("Failed to reattach the driver to kernel: (%d) %s\n", res, libusb_error_name(res)); + dev->is_driver_detached = 0; + } +#endif + return 0; + } return 1; } diff --git a/libusb/hidapi_libusb_report_descriptor.h b/libusb/hidapi_libusb_report_descriptor.h index 144abf60e..fd6d41564 100644 --- a/libusb/hidapi_libusb_report_descriptor.h +++ b/libusb/hidapi_libusb_report_descriptor.h @@ -20,6 +20,9 @@ struct report_global_state { }; #define REPORT_GLOBAL_STACK_SIZE 16 +/* Bound descriptor-controlled allocations to a conservative 16-bit report + range while leaving room for practical libusb interrupt transfers. */ +#define HIDAPI_LIBUSB_MAX_REPORT_SIZE ((size_t)UINT16_MAX) static uint32_t get_report_item_data(const uint8_t *report_descriptor, size_t item_offset, size_t data_len) { @@ -77,7 +80,7 @@ static ssize_t get_max_report_size(const uint8_t *report_descriptor, size_t desc state.report_size_set = 1; break; case 0x84: /* Report ID */ - if (data_len != 1 || value == 0) + if (data_len == 0 || value == 0 || value > UINT8_MAX) return -1; state.report_id = (uint8_t)value; report_ids_used = 1; @@ -140,14 +143,20 @@ static ssize_t get_max_report_size(const uint8_t *report_descriptor, size_t desc if (max_bits > SIZE_MAX - 7) return -1; max_bytes = (max_bits + 7) / 8; - if (max_bytes >= (size_t)PTRDIFF_MAX) + if (max_bytes >= (size_t)PTRDIFF_MAX || + max_bytes >= HIDAPI_LIBUSB_MAX_REPORT_SIZE) return -1; return (ssize_t)(max_bytes + 1); } if (report_bits[0] > (size_t)PTRDIFF_MAX - 7) return -1; - return (ssize_t)((report_bits[0] + 7) / 8); + { + const size_t report_size = (report_bits[0] + 7) / 8; + if (report_size > HIDAPI_LIBUSB_MAX_REPORT_SIZE) + return -1; + return (ssize_t)report_size; + } } #endif diff --git a/libusb/test/CMakeLists.txt b/libusb/test/CMakeLists.txt index b222c3741..436fc0ded 100644 --- a/libusb/test/CMakeLists.txt +++ b/libusb/test/CMakeLists.txt @@ -38,7 +38,9 @@ set(HID_REPORT_DESCRIPTOR_TEST_CASES 1532_00A3_0002_0001 ) -set(CMAKE_VERSION_SUPPORTS_ENVIRONMENT_MODIFICATION "3.22") +add_test(NAME LibUsbHidReportDescriptorParserTest + COMMAND max_input_report_size_test --self-test +) foreach(TEST_CASE ${HID_REPORT_DESCRIPTOR_TEST_CASES}) set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data") diff --git a/libusb/test/max_input_report_size_test.c b/libusb/test/max_input_report_size_test.c index 6b95e93bf..c7fc26a46 100644 --- a/libusb/test/max_input_report_size_test.c +++ b/libusb/test/max_input_report_size_test.c @@ -124,21 +124,35 @@ static int parse_c_hex_bytes(char *line, unsigned char *data, size_t data_size, if (comment) *comment = '\0'; - while ((cursor = strstr(cursor, "0x")) != NULL) { - if (cursor[2] != '\0' && - cursor[3] != '\0' && - isxdigit((unsigned char)cursor[2]) && - isxdigit((unsigned char)cursor[3]) && - !isxdigit((unsigned char)cursor[4])) { - char byte_text[3] = {cursor[2], cursor[3], '\0'}; - unsigned int value = (unsigned int)strtoul(byte_text, NULL, 16); - - if (append_byte(data, data_size, data_length, value, filename) < 0) - return -1; - found = 1; - cursor += 4; - } else { - cursor += 2; + while (isspace((unsigned char)*cursor)) + cursor++; + if (cursor[0] != '0' || cursor[1] != 'x') + return 0; + + while (*cursor) { + char *end; + unsigned long value; + + if (cursor[0] != '0' || cursor[1] != 'x') { + fprintf(stderr, "Malformed C hex byte list in '%s'\n", filename); + return -1; + } + value = strtoul(cursor + 2, &end, 16); + if (end == cursor + 2 || end - (cursor + 2) > 2 || + append_byte(data, data_size, data_length, (unsigned int)value, filename) < 0) + return -1; + found = 1; + cursor = end; + + while (isspace((unsigned char)*cursor)) + cursor++; + if (*cursor == ',') { + cursor++; + while (isspace((unsigned char)*cursor)) + cursor++; + } else if (*cursor != '\0') { + fprintf(stderr, "Malformed C hex byte list in '%s'\n", filename); + return -1; } } @@ -256,6 +270,10 @@ static bool read_report_descriptor(const char *filename, unsigned char *data, si static int test_report_descriptor_parser(void) { + char single_digit_hex[] = " 0x5, 0x0a, // valid C-style byte list"; + char stray_hex_text[] = "description mentions 0x05 but is not a byte list"; + unsigned char fixture_bytes[2]; + size_t fixture_byte_count = 0; static const uint8_t missing_report_size[] = {0x95, 0x01, 0x81, 0x00}; static const uint8_t truncated_item[] = {0x75}; static const uint8_t output_only[] = {0x75, 0x08, 0x95, 0x01, 0x91, 0x00}; @@ -264,7 +282,48 @@ static int test_report_descriptor_parser(void) 0x85, 0x02, 0x95, 0x01, 0x81, 0x00, 0x85, 0x01, 0x95, 0x02, 0x81, 0x00, }; + static const uint8_t push_pop[] = { + 0x75, 0x08, 0x95, 0x01, 0xa4, + 0x75, 0x10, 0x95, 0x02, 0x81, 0x00, + 0xb4, 0x81, 0x00, + }; + static const uint8_t long_item[] = { + 0x75, 0x08, 0xfe, 0x02, 0x99, 0xaa, 0xbb, + 0x95, 0x03, 0x81, 0x00, + }; + static const uint8_t four_byte_globals[] = { + 0x77, 0x08, 0x00, 0x00, 0x00, + 0x97, 0x02, 0x00, 0x00, 0x00, + 0x81, 0x00, + }; + static const uint8_t two_byte_report_id[] = { + 0x86, 0x01, 0x00, 0x75, 0x08, 0x95, 0x01, 0x81, 0x00, + }; + static const uint8_t mixed_report_id_zero[] = { + 0x75, 0x08, 0x95, 0x01, 0x81, 0x00, + 0x85, 0x01, 0x81, 0x00, + }; + static const uint8_t oversized_report[] = { + 0x75, 0x20, 0x97, 0xff, 0xff, 0xff, 0x7f, 0x81, 0x00, + }; + static const uint8_t maximum_report[] = { + 0x75, 0x08, 0x97, 0xff, 0xff, 0x00, 0x00, 0x81, 0x00, + }; + static const uint8_t over_maximum_report[] = { + 0x75, 0x08, 0x97, 0x00, 0x00, 0x01, 0x00, 0x81, 0x00, + }; + static const uint8_t accumulated_overflow[] = { + 0x77, 0xff, 0xff, 0xff, 0xff, + 0x97, 0xff, 0xff, 0xff, 0xff, + 0x81, 0x00, 0x81, 0x00, + }; + if (parse_c_hex_bytes(single_digit_hex, fixture_bytes, sizeof(fixture_bytes), &fixture_byte_count, "self-test") != 1 || + fixture_byte_count != 2 || fixture_bytes[0] != 0x05 || fixture_bytes[1] != 0x0a || + parse_c_hex_bytes(stray_hex_text, fixture_bytes, sizeof(fixture_bytes), &fixture_byte_count, "self-test") != 0) { + fprintf(stderr, "C-style report descriptor fixture parsing failed\n"); + return -1; + } if (get_max_report_size(missing_report_size, sizeof(missing_report_size), REPORT_DESCR_INPUT) != -1 || get_max_report_size(truncated_item, sizeof(truncated_item), REPORT_DESCR_INPUT) != -1) { fprintf(stderr, "Malformed report descriptor was not rejected\n"); @@ -278,6 +337,21 @@ static int test_report_descriptor_parser(void) fprintf(stderr, "Repeated report ID fields were not accumulated correctly\n"); return -1; } + if (get_max_report_size(push_pop, sizeof(push_pop), REPORT_DESCR_INPUT) != 5 || + get_max_report_size(long_item, sizeof(long_item), REPORT_DESCR_INPUT) != 3 || + get_max_report_size(four_byte_globals, sizeof(four_byte_globals), REPORT_DESCR_INPUT) != 2 || + get_max_report_size(two_byte_report_id, sizeof(two_byte_report_id), REPORT_DESCR_INPUT) != 2 || + get_max_report_size(maximum_report, sizeof(maximum_report), REPORT_DESCR_INPUT) != (ssize_t)HIDAPI_LIBUSB_MAX_REPORT_SIZE) { + fprintf(stderr, "Valid global-item encodings were not parsed correctly\n"); + return -1; + } + if (get_max_report_size(mixed_report_id_zero, sizeof(mixed_report_id_zero), REPORT_DESCR_INPUT) != -1 || + get_max_report_size(oversized_report, sizeof(oversized_report), REPORT_DESCR_INPUT) != -1 || + get_max_report_size(over_maximum_report, sizeof(over_maximum_report), REPORT_DESCR_INPUT) != -1 || + get_max_report_size(accumulated_overflow, sizeof(accumulated_overflow), REPORT_DESCR_INPUT) != -1) { + fprintf(stderr, "Invalid or unsafe report descriptor was not rejected\n"); + return -1; + } return 0; } @@ -293,6 +367,9 @@ int main(int argc, char *argv[]) ssize_t feature_size; int ret = EXIT_SUCCESS; + if (argc == 2 && strcmp(argv[1], "--self-test") == 0) + return test_report_descriptor_parser() == 0 ? EXIT_SUCCESS : EXIT_FAILURE; + if (argc != 3) { fprintf(stderr, "Expected 2 arguments ('<>.pp_data' and '<>_real.rpt_desc'), got: %d\n", argc - 1); return EXIT_FAILURE; @@ -300,8 +377,6 @@ int main(int argc, char *argv[]) printf("Checking: '%s' / '%s'\n", argv[1], argv[2]); - if (test_report_descriptor_parser() < 0) - return EXIT_FAILURE; if (!read_report_descriptor(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) return EXIT_FAILURE; if (parse_expected_report_sizes(argv[1], &expected) < 0)