-
Notifications
You must be signed in to change notification settings - Fork 429
Properly determine libusb read size for large reports (Fixes #274) #728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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 | ||||||||||||||
|
@@ -95,6 +101,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 +143,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 +285,81 @@ 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; | ||||||||||||||
|
||||||||||||||
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 == report_type) { /* Input / Output / Feature */ | ||||||||||||||
if (report_count < 0 || report_size < 0) { | ||||||||||||||
/* We are missing size or count. That isn't good. */ | ||||||||||||||
return 0; | ||||||||||||||
Comment on lines
+332
to
+334
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This would mean a corrupt ReportDescriptor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't consistent with whether I was using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense! |
||||||||||||||
} | ||||||||||||||
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; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
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. */ | ||||||||||||||
return ((max_size + 7) / 8) + 1; | ||||||||||||||
} | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
#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 +1108,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); | ||||||||||||||
|
@@ -1217,6 +1308,15 @@ 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); | ||||||||||||||
} else { | ||||||||||||||
dev->max_input_report_size = 0; | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
dev->input_endpoint = 0; | ||||||||||||||
dev->input_ep_max_packet_size = 0; | ||||||||||||||
dev->output_endpoint = 0; | ||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -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: | ||||
# <name>.pp_data - textual representation of HIDP_PREPARSED_DATA; | ||||
# <name>_expected.rpt_desc - reconstructed HID Report Descriptor out of .pp_data file; | ||||
# | ||||
# (Non-required by test): | ||||
# <name>_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") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe move the test data to |
||||
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") | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to parse the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. I assumed that the The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you can put them into https://eleccelerator.com/usbdescreqparser/ to unify the format. That makes them also human readable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea. |
||||
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 "$<TARGET_FILE_DIR:hidapi_winapi>" | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
) | ||||
endforeach() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,142 @@ | ||
#include <stddef.h> | ||
#include <stdio.h> | ||
#include <stdlib.h> | ||
#include <stdbool.h> | ||
#include <string.h> | ||
#include <errno.h> | ||
|
||
#include "../hid.c" | ||
|
||
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) { | ||
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) { | ||
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; | ||
} | ||
} | ||
} | ||
|
||
fclose(file); | ||
|
||
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) | ||
{ | ||
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; | ||
} | ||
|
||
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; | ||
} | ||
|
||
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; | ||
|
||
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; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also need to update builds.yml