hid_error(NULL) hands out an unsynchronized pointer to the global error string #827

New issue

Open

opened 2026-07-15 07:06:21 -05:00 by Youw · 0 comments

Youw commented

2026-07-15 07:06:21 -05:00

(Migrated from github.com)

hid_error(NULL) returns last_global_error_str (the process-global error string) directly, without holding any lock, while another thread can free()-and-replace it. Any HIDAPI-owned internal thread that writes the global error therefore races an application's hid_error(NULL) read into a use-after-free — and the application has no way to serialize against a thread it does not own.

The hotplug work (#822–#825) sidesteps this by making HIDAPI's internal threads never write the global error string (now stated in hidapi.h), so the hotplug feature itself is safe. But the underlying reader/writer hazard on last_global_error_str is pre-existing and broader than hotplug: two application threads that concurrently call an error-setting API and hid_error(NULL) already race, and a writer-side lock cannot close it because the reader escapes with a raw internal pointer.

A proper fix (likely for v1.0) is either thread-local error state, or a hid_error(NULL) that copies into a caller-owned / thread-local buffer instead of returning an internal pointer. Filing so it is not lost.

Drafted with Claude Code.

`hid_error(NULL)` returns `last_global_error_str` (the process-global error string) directly, without holding any lock, while another thread can `free()`-and-replace it. Any HIDAPI-owned internal thread that writes the global error therefore races an application's `hid_error(NULL)` read into a use-after-free — and the application has no way to serialize against a thread it does not own. The hotplug work (#822–#825) sidesteps this by making HIDAPI's internal threads **never** write the global error string (now stated in `hidapi.h`), so the hotplug feature itself is safe. But the underlying reader/writer hazard on `last_global_error_str` is **pre-existing and broader than hotplug**: two application threads that concurrently call an error-setting API and `hid_error(NULL)` already race, and a writer-side lock cannot close it because the reader escapes with a raw internal pointer. A proper fix (likely for v1.0) is either thread-local error state, or a `hid_error(NULL)` that copies into a caller-owned / thread-local buffer instead of returning an internal pointer. Filing so it is not lost. _Drafted with Claude Code._