Skip to content

Commit cc60780

Browse files
committed
mac: suppress global-error writes on the hotplug event thread
Honor the cross-backend contract (documented in hidapi.h and already enforced by the libusb and linux backends) that HIDAPI calls made from within a hotplug callback do not update the global error string: the callback runs on HIDAPI's internal event thread, so such a write races an application's hid_error(NULL) read - a use-after-free of last_global_error_str, the same class as the original hotplug blocker. The event thread now publishes its pthread id (guarded by the leaf global_error_mutex) as its first action and clears it in its epilogue. register_global_error()[_format]() skip the write when invoked on that thread, covering both the failure paths and the success-path clear of a callback that re-enters hid_hotplug_(de)register_callback(). Writes from application threads are unaffected, and per-device errors are never suppressed. Assisted-by: claude-code:claude-opus-4-8
1 parent 98d2354 commit cc60780

1 file changed

Lines changed: 66 additions & 2 deletions

File tree

mac/hid.c

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,12 @@ static void register_error_str_vformat(wchar_t **error_str, const char *format,
262262
register_error_str(error_str, msg);
263263
}
264264

265+
/* True when the calling thread is HIDAPI's internal hotplug event thread; used
266+
to suppress writes to the global error string made from that thread (see the
267+
definition after the hotplug context for the full rationale). Must be called
268+
with global_error_mutex held. */
269+
static int hid_internal_on_event_thread(void);
270+
265271
/* Serializes the mutations of the global error string: the hotplug API is
266272
thread-safe and its failure paths (and the implicit hid_init()) may write
267273
the global error from multiple threads concurrently. */
@@ -275,7 +281,14 @@ static pthread_mutex_t global_error_mutex = PTHREAD_MUTEX_INITIALIZER;
275281
static void register_global_error(const char *msg)
276282
{
277283
pthread_mutex_lock(&global_error_mutex);
278-
register_error_str(&last_global_error_str, msg);
284+
/* Honor the cross-backend contract (see hidapi.h): a global-error write
285+
attempted on the internal hotplug event thread - e.g. from a
286+
hid_hotplug_(de)register_callback() call re-entered from within a user
287+
callback - must not touch the global error string. Per-device errors go
288+
through register_error_str() with a different target and are unaffected;
289+
only this process-global string is suppressed. */
290+
if (!hid_internal_on_event_thread())
291+
register_error_str(&last_global_error_str, msg);
279292
pthread_mutex_unlock(&global_error_mutex);
280293
}
281294

@@ -285,7 +298,9 @@ static void register_global_error_format(const char *format, ...)
285298
va_list args;
286299
va_start(args, format);
287300
pthread_mutex_lock(&global_error_mutex);
288-
register_error_str_vformat(&last_global_error_str, format, args);
301+
/* See register_global_error(): suppressed on the internal event thread. */
302+
if (!hid_internal_on_event_thread())
303+
register_error_str_vformat(&last_global_error_str, format, args);
289304
pthread_mutex_unlock(&global_error_mutex);
290305
va_end(args);
291306
}
@@ -588,6 +603,15 @@ static struct hid_hotplug_context {
588603
registering thread after it (the barrier is the synchronization edge) */
589604
unsigned char startup_ok;
590605

606+
/* Identity of the running hotplug event thread. Published by that thread as
607+
its first action and cleared in its epilogue, so it is valid exactly while
608+
an event thread exists. Guarded by global_error_mutex (a leaf mutex), NOT
609+
the hotplug mutex: the global-error writer consults it while holding
610+
global_error_mutex and must never take the hotplug mutex it may already
611+
hold. Read only via hid_internal_on_event_thread(). */
612+
pthread_t event_thread_id;
613+
unsigned char event_thread_id_valid;
614+
591615
/* Linked list of the hotplug callbacks */
592616
struct hid_hotplug_callback *hotplug_cbs;
593617

@@ -600,6 +624,24 @@ static struct hid_hotplug_context {
600624
can ever lock a mutex that hid_exit() destroyed underneath it */
601625
static pthread_once_t hid_hotplug_init_once = PTHREAD_ONCE_INIT;
602626

627+
/* HIDAPI's public API contract (see hidapi.h) is that HIDAPI calls made from
628+
within a hotplug callback do not update the global error string: the callback
629+
runs on this internal event thread, and an application cannot serialize a
630+
hid_error(NULL) read against a write from that thread - that would be a
631+
use-after-free of last_global_error_str. This mirrors the libusb and linux
632+
backends, which likewise suppress such writes. A callback may re-enter the
633+
public hid_hotplug_register_callback()/hid_hotplug_deregister_callback(),
634+
whose success and failure paths both write the global error; those writes are
635+
suppressed via this check in register_global_error()[_format]().
636+
Returns non-zero when the caller is the hotplug event thread. Must be called
637+
with global_error_mutex held (the event_thread_id* fields are guarded by it),
638+
which the global-error writer already holds. */
639+
static int hid_internal_on_event_thread(void)
640+
{
641+
return hid_hotplug_context.event_thread_id_valid
642+
&& pthread_equal(pthread_self(), hid_hotplug_context.event_thread_id);
643+
}
644+
603645
static void hid_internal_hotplug_remove_postponed(void)
604646
{
605647
/* Unregister the callbacks whose removal was postponed */
@@ -1661,6 +1703,16 @@ static void hid_internal_hotplug_thread_epilogue(void)
16611703
{
16621704
pthread_mutex_lock(&hid_hotplug_context.mutex);
16631705

1706+
/* The event thread is exiting: stop suppressing global-error writes for its
1707+
pthread id. Cleared under the hotplug mutex - before the thread is detached
1708+
or collected, and thus before any replacement event thread can be started
1709+
and publish its own id - so a later thread's id can never be clobbered.
1710+
Ordering is hotplug mutex -> global_error_mutex, the same order the
1711+
global-error writer uses when it is called under the hotplug mutex. */
1712+
pthread_mutex_lock(&global_error_mutex);
1713+
hid_hotplug_context.event_thread_id_valid = 0;
1714+
pthread_mutex_unlock(&global_error_mutex);
1715+
16641716
if (hid_hotplug_context.thread_needs_join && !hid_hotplug_context.join_in_progress) {
16651717
/* Nobody is inside pthread_join() on this thread, and nobody can enter
16661718
it any more: the decision is taken under the mutex on both sides (see
@@ -1682,6 +1734,18 @@ static void* hotplug_thread(void* user_data)
16821734

16831735
(void) user_data;
16841736

1737+
/* Publish this thread's identity as the very first action, before anything
1738+
here can attempt a global-error write, so that any such write on this
1739+
internal event thread - notably from a user callback that re-enters
1740+
hid_hotplug_(de)register_callback() - is suppressed (see
1741+
hid_internal_on_event_thread()). Uses global_error_mutex only: the hotplug
1742+
mutex must not be taken during the startup phase (the registrant holds it,
1743+
parked at the startup barrier). */
1744+
pthread_mutex_lock(&global_error_mutex);
1745+
hid_hotplug_context.event_thread_id = pthread_self();
1746+
hid_hotplug_context.event_thread_id_valid = 1;
1747+
pthread_mutex_unlock(&global_error_mutex);
1748+
16851749
/* Startup phase: the registering thread holds the hotplug mutex and is
16861750
parked at the startup barrier, so this thread has exclusive access to the
16871751
context - and it MUST NOT take the mutex until the barrier has been passed

0 commit comments

Comments
 (0)