diff --git a/docs/libusb-input-report-overflow.md b/docs/libusb-input-report-overflow.md new file mode 100644 index 000000000..acf1dbd0c --- /dev/null +++ b/docs/libusb-input-report-overflow.md @@ -0,0 +1,96 @@ +# Reproducing silent HID input-report loss in HIDAPI's libusb backend + +## Issue + +The libusb backend queues HID input reports in a linked list. When the queue reaches approximately 32 entries, `read_callback()` silently discards an older report with `return_data(dev, NULL, 0)`. Applications cannot distinguish a full transfer from one missing HID reports unless their application protocol has an independent integrity check. + +The original implementation repeatedly traverses the queue to append at its tail; this is separate from the primary data-loss issue. + +## Reproduction + +Test platform: ThinkPad T400 running Linux; Abbott FreeStyle Libre 2 USB HID reader; `hidapi` Python package 0.15.0 compiled using its libusb backend. Python 3.14. The reader communicates over full-speed USB, tested on an EHCI hub. + +The original libusb queue was instrumented with: +- `reports_received`: incoming reports queued +- `reports_dropped`: reports removed due to queue overflow +- `queue_max`: highest observed queue occupancy + +Instrumentation used the existing receive-thread mutex and logged the results at `hid_close()`. + +Results: + +| Queue | Observed statistics | Result | +| --- | --- | --- | +| Original (~32), light load | received=22658, dropped=0, max_queue=4 | Complete transfer | +| Original (~32), moderate desktop activity | received=22659, dropped=0, max_queue=27 | Complete transfer | +| Original (~32), under activity | received=12365, dropped=92, max_queue=33 | Historical transfer checksum failure | +| Experimental (~1024) | received=22658, dropped=0, max_queue=88 | Complete transfer | +| Experimental (~1024), heavier activity | received=22659, dropped=0, max_queue=145 | Complete transfer | +| Experimental (~1024), end-to-end validation | received=22658, dropped=0, max_queue=84 | Complete transfer | + +The failed historical transfer contained 12,269 HID reports; a complete historical transfer contained 12,361. The difference of **92 reports exactly matched the native overflow count**. + +A successful end-to-end collector run was about 33 seconds, read 12,328 historical records and 5,032 scan events, and confirmed counters `received=22658 dropped=0 max_queue=84` both from Python and C upon close. + +No patient information, device serial numbers, or patient-level records are included here. + +## Locally validated mitigation + +The validated experimental implementation changes the overflow threshold in the vendored HIDAPI 0.15.0 libusb backend from `num_queued > 30` to `num_queued > 1022` (approximately 1,024 queued reports). It also exposes queue counters through a temporary `hid_get_queue_stats()` C API and `hid.device.get_queue_stats()` Python binding. + +The collector rejects the read if the native statistics API is unavailable or if `reports_dropped > 0`. Application-level checksums remain enabled. A 1,024-report queue is not a guarantee against all scheduling delays; the overflow counter is essential. + +## Intended upstream contributions + +- C HIDAPI: port the **experimentally validated** queue threshold and native queue statistics to the maintained libusb backend. +- Python binding (`trezor/cython-hidapi`): expose `get_queue_stats()` on `hid.device`. + +The work is split across two upstream repositories; the local experimental build combines both changes. The documentation in this branch records measured behavior; it does **not** claim upstream CI or current upstream builds have been tested. + +## Source of truth + +The local experimental source files under `essais-hidapi/hidapi-0.15.0` on the development workstation are the reference implementation. Do not substitute a reimplementation when preparing patches. + +## API and compatibility + +`hid_get_queue_stats(dev, &received, &dropped, &max_queue)` returns zero +for an open libusb device. All pointers are required; invalid arguments return +-1 without changing outputs. The existing receive mutex protects the snapshot. +Other backends export the same function and return -1 without changing outputs. +`HID_API_HAS_QUEUE_STATS` identifies headers declaring this API. + +The 1024-report limit preserves the tested oldest-report discard policy. +`max_queue` measures depth before overflow removal and can therefore reach 1025. +Counters are cumulative for the lifetime of an open device and do not reset +when read. Closing a libusb device logs the native counters to standard error. +Callers must serialize close against statistics reads, as for other device APIs. + +## Hardware-free regression + +From the repository root, compile and run sequentially: + +```sh +mkdir -p build-local +cc -g -fsanitize=address,undefined -pthread -Ihidapi -Ilibusb \ + $(pkg-config --cflags libusb-1.0) tests/queue-stats/test_queue_stats.c \ + -o build-local/test-queue-stats $(pkg-config --libs libusb-1.0) +./build-local/test-queue-stats +``` + +The test uses the actual receive callback and queue with mocked USB initialization, +shutdown and transfer submission. It verifies invalid arguments, initial counters, +the 1024/1025 overflow boundary, FIFO retention of the newest 1024 reports, +and cumulative counters after 22658 synthetic reports. It never discovers or +opens a USB device. + +## Contribution build validation + +Linux x86_64, GCC 15.2, libusb 1.0.29: libusb and hidraw shared libraries +compiled sequentially. The synthetic callback regression passed under AddressSanitizer +and UndefinedBehaviorSanitizer, including a separate 22658-report zero-drop run. +Leak detection was disabled (`ASAN_OPTIONS=detect_leaks=0`) because LeakSanitizer +cannot operate under this environment's tracing restrictions. The hidraw unsupported +API returned -1 with outputs unchanged. No devices were enumerated or opened. +CMake and Autotools are unavailable here, so native libraries were compiled directly +with `cc -shared -fPIC` and the appropriate pkg-config flags. Windows, macOS and +NetBSD changes were inspected but require their platform CI for compilation. diff --git a/hidapi/hidapi.h b/hidapi/hidapi.h index cbc3107dd..a65621e73 100644 --- a/hidapi/hidapi.h +++ b/hidapi/hidapi.h @@ -547,6 +547,22 @@ extern "C" { */ void HID_API_EXPORT HID_API_CALL hid_close(hid_device *dev); + /** @brief Read native libusb receive queue diagnostics. + * + * received counts reports queued since opening the device, including + * reports subsequently dropped. dropped counts oldest reports removed + * on overflow. max_queue is the peak depth before overflow removal, + * so it may reach 1025 with the 1024-report queue limit. + * The three counters are read under the existing receive mutex. + * All output pointers must be non-NULL. Outputs are unchanged on error. + * @returns 0 on success, -1 for invalid parameters or an unsupported + * backend. Only the libusb backend supplies these counters. + */ +#define HID_API_HAS_QUEUE_STATS 1 + int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue); + /** @brief Get The Manufacturer String from a HID device. @ingroup API diff --git a/libusb/hid.c b/libusb/hid.c index 74f59cf5e..598aa51f6 100644 --- a/libusb/hid.c +++ b/libusb/hid.c @@ -136,6 +136,11 @@ struct hid_device_ { /* List of received input reports. */ struct input_report *input_reports; + /* Receive queue diagnostics (protected by thread_state mutex). */ + unsigned long long reports_received; + unsigned long long reports_dropped; + unsigned int queue_max; + /* Was kernel driver detached by libusb */ #ifdef DETACH_KERNEL_DRIVER int is_driver_detached; @@ -162,6 +167,21 @@ static hidapi_error_ctx last_global_error; uint16_t get_usb_code_for_current_locale(void); static int return_data(hid_device *dev, unsigned char *data, size_t length); +/* Native receive queue counters. */ +int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue) +{ + if (!dev || !received || !dropped || !max_queue) + return -1; + hidapi_thread_mutex_lock(&dev->thread_state); + *received = dev->reports_received; + *dropped = dev->reports_dropped; + *max_queue = dev->queue_max; + hidapi_thread_mutex_unlock(&dev->thread_state); + return 0; +} + static hid_device *new_hid_device(void) { hid_device *dev = (hid_device*) calloc(1, sizeof(hid_device)); @@ -1120,6 +1140,9 @@ static void LIBUSB_CALL read_callback(struct libusb_transfer *transfer) if (dev->input_reports == NULL) { /* The list is empty. Put it at the root. */ dev->input_reports = rpt; + dev->reports_received++; + if (dev->queue_max < 1) + dev->queue_max = 1; hidapi_thread_cond_signal(&dev->thread_state); } else { @@ -1131,11 +1154,15 @@ static void LIBUSB_CALL read_callback(struct libusb_transfer *transfer) num_queued++; } cur->next = rpt; - - /* Pop one off if we've reached 30 in the queue. This - way we don't grow forever if the user never reads - anything from the device. */ - if (num_queued > 30) { + dev->reports_received++; + unsigned int queue_depth = (unsigned int)num_queued + 2; + if (queue_depth > dev->queue_max) + dev->queue_max = queue_depth; + + /* Preserve the validated 1,024-report queue limit; prevent + unbounded memory growth if the application never reads. */ + if (num_queued > 1022) { + dev->reports_dropped++; return_data(dev, NULL, 0); } } @@ -1990,6 +2017,9 @@ void HID_API_EXPORT hid_close(hid_device *dev) /* Close the handle */ libusb_close(dev->device_handle); + fprintf(stderr, "HIDAPI-STATS TEST-1024: received=%llu dropped=%llu max_queue=%u\n", + dev->reports_received, dev->reports_dropped, dev->queue_max); + /* Clear out the queue of received reports. */ hidapi_thread_mutex_lock(&dev->thread_state); while (dev->input_reports) { diff --git a/linux/hid.c b/linux/hid.c index 54165661c..a23e95097 100644 --- a/linux/hid.c +++ b/linux/hid.c @@ -1449,3 +1449,15 @@ HID_API_EXPORT const wchar_t * HID_API_CALL hid_error(hid_device *dev) return L"Success"; return last_global_error_str; } + +/* These receive queue counters belong to the libusb backend only. */ +int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue) +{ + (void)dev; + (void)received; + (void)dropped; + (void)max_queue; + return -1; +} diff --git a/mac/hid.c b/mac/hid.c index 4ec659523..d25159ed3 100644 --- a/mac/hid.c +++ b/mac/hid.c @@ -1596,3 +1596,15 @@ HID_API_EXPORT const wchar_t * HID_API_CALL hid_error(hid_device *dev) return L"Success"; return last_global_error_str; } + +/* These receive queue counters belong to the libusb backend only. */ +int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue) +{ + (void)dev; + (void)received; + (void)dropped; + (void)max_queue; + return -1; +} diff --git a/netbsd/hid.c b/netbsd/hid.c index a9fca67c5..f946ddf31 100644 --- a/netbsd/hid.c +++ b/netbsd/hid.c @@ -1213,3 +1213,15 @@ HID_API_EXPORT const char* HID_API_CALL hid_version_str(void) { return HID_API_VERSION_STR; } + +/* These receive queue counters belong to the libusb backend only. */ +int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue) +{ + (void)dev; + (void)received; + (void)dropped; + (void)max_queue; + return -1; +} diff --git a/tests/queue-stats/test_queue_stats.c b/tests/queue-stats/test_queue_stats.c new file mode 100644 index 000000000..8986210a5 --- /dev/null +++ b/tests/queue-stats/test_queue_stats.c @@ -0,0 +1,72 @@ +/* Hardware-free regression: exercise the actual libusb callback and queue. */ +#include +#include +static int LIBUSB_CALL fake_submit(struct libusb_transfer *transfer) +{ + (void)transfer; + return 0; +} +/* Avoid USB discovery entirely, including during library initialization. */ +static int LIBUSB_CALL fake_init(libusb_context **context) +{ + *context = (libusb_context *)&fake_init; + return 0; +} +static void LIBUSB_CALL fake_exit(libusb_context *context) { (void)context; } +#define libusb_init fake_init +#define libusb_exit fake_exit +#define libusb_submit_transfer fake_submit +#include "../../libusb/hid.c" + +int main(void) +{ + hid_device *dev; + unsigned long long received = 99, dropped = 99; + unsigned int peak = 99; + unsigned char payload[2], output[2]; + struct libusb_transfer transfer = {0}; + assert(hid_init() == 0); + dev = new_hid_device(); + assert(dev); + assert(hid_get_queue_stats(dev, &received, &dropped, &peak) == 0); + assert(received == 0 && dropped == 0 && peak == 0); + assert(hid_get_queue_stats(NULL, &received, &dropped, &peak) == -1); + assert(hid_get_queue_stats(dev, NULL, &dropped, &peak) == -1); + assert(hid_get_queue_stats(dev, &received, NULL, &peak) == -1); + assert(hid_get_queue_stats(dev, &received, &dropped, NULL) == -1); + transfer.user_data = dev; + transfer.status = LIBUSB_TRANSFER_COMPLETED; + transfer.buffer = payload; + transfer.actual_length = sizeof(payload); + for (unsigned int i = 0; i < 1025; ++i) { + payload[0] = i & 255; + payload[1] = i >> 8; + read_callback(&transfer); + } + assert(hid_get_queue_stats(dev, &received, &dropped, &peak) == 0); + assert(received == 1025 && dropped == 1 && peak == 1025); + for (unsigned int i = 1; i < 1025; ++i) { + assert(return_data(dev, output, sizeof(output)) == 2); + assert(output[0] == (i & 255) && output[1] == (i >> 8)); + } + assert(dev->input_reports == NULL); + for (unsigned int i = 1025; i < 22658; ++i) { + read_callback(&transfer); + assert(return_data(dev, output, sizeof(output)) == 2); + } + assert(hid_get_queue_stats(dev, &received, &dropped, &peak) == 0); + assert(received == 22658 && dropped == 1 && peak == 1025); + free_hid_device(dev); + dev = new_hid_device(); + assert(dev); + transfer.user_data = dev; + for (unsigned int i = 0; i < 22658; ++i) { + read_callback(&transfer); + assert(return_data(dev, output, sizeof(output)) == 2); + } + assert(hid_get_queue_stats(dev, &received, &dropped, &peak) == 0); + assert(received == 22658 && dropped == 0 && peak == 1); + free_hid_device(dev); + assert(hid_exit() == 0); + return 0; +} diff --git a/windows/hid.c b/windows/hid.c index 705847e0b..78ecf5974 100644 --- a/windows/hid.c +++ b/windows/hid.c @@ -1616,3 +1616,15 @@ HID_API_EXPORT const wchar_t * HID_API_CALL hid_error(hid_device *dev) #ifdef __cplusplus } /* extern "C" */ #endif + +/* These receive queue counters belong to the libusb backend only. */ +int HID_API_EXPORT HID_API_CALL hid_get_queue_stats( + hid_device *dev, unsigned long long *received, + unsigned long long *dropped, unsigned int *max_queue) +{ + (void)dev; + (void)received; + (void)dropped; + (void)max_queue; + return -1; +}