Skip to content

Commit 09143cc

Browse files
Youwclaude
andcommitted
Fix devs cleanup: protect teardown under hid_hotplug_context.mutex
The struct comment on hid_hotplug_context.devs claimed it was protected by hid_hotplug_context.mutex, but the final cleanup in callback_thread ran under only callback_thread.mutex — inconsistent with all other accesses (process_hotplug_event, hid_hotplug_register_callback) that hold hid_hotplug_context.mutex when reading or writing devs. Move the free to hid_internal_hotplug_cleanup(), after hidapi_thread_join(libusb_thread) returns, so every access to devs — including the teardown free — happens while holding the context mutex. Also fix a leak on the hid_hotplug_register_callback error path: when libusb_hotplug_register_callback fails, no hotplug thread is created, so hid_internal_hotplug_cleanup() can't be used to unwind the devs list assigned by the preceding hid_enumerate() call. Free it inline on that path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 7223ef7 commit 09143cc

1 file changed

Lines changed: 13 additions & 7 deletions

File tree

libusb/hid.c

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,9 @@ static struct hid_hotplug_context {
173173
struct hid_hotplug_callback *hotplug_cbs;
174174

175175
/* Linked list of the device infos (mandatory when the device is disconnected).
176-
* Protected by `mutex` for both reads and writes.
177-
* Final cleanup is done by callback_thread during shutdown. */
176+
* Protected by `mutex`: all reads, writes and the final free during teardown
177+
* are performed while holding it. The teardown free runs in
178+
* hid_internal_hotplug_cleanup() after the hotplug threads have been joined. */
178179
struct hid_device_info *devs;
179180
} hid_hotplug_context = {
180181
.next_handle = FIRST_HOTPLUG_CALLBACK_HANDLE,
@@ -586,6 +587,11 @@ static void hid_internal_hotplug_cleanup()
586587

587588
/* Wait for both threads to stop */
588589
hidapi_thread_join(&hid_hotplug_context.libusb_thread);
590+
591+
/* Both hotplug threads have exited: we now have exclusive access to `devs`
592+
* (the caller holds `mutex` and no hotplug event can reach process_hotplug_event). */
593+
hid_free_enumeration(hid_hotplug_context.devs);
594+
hid_hotplug_context.devs = NULL;
589595
}
590596

591597
static void hid_internal_hotplug_init()
@@ -1250,10 +1256,6 @@ static void* callback_thread(void* user_data)
12501256
}
12511257
}
12521258

1253-
/* Cleanup connected device list */
1254-
hid_free_enumeration(hid_hotplug_context.devs);
1255-
hid_hotplug_context.devs = NULL;
1256-
12571259
hidapi_thread_mutex_unlock(&hid_hotplug_context.callback_thread);
12581260

12591261
return NULL;
@@ -1362,8 +1364,12 @@ int HID_API_EXPORT HID_API_CALL hid_hotplug_register_callback(unsigned short ven
13621364
LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT,
13631365
0, LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY, LIBUSB_HOTPLUG_MATCH_ANY, &hid_libusb_hotplug_callback, NULL,
13641366
&hid_hotplug_context.callback_handle)) {
1365-
/* Major malfunction, failed to register a callback */
1367+
/* Major malfunction, failed to register a callback.
1368+
* No hotplug thread was started, so we must unwind `devs` ourselves
1369+
* (hid_internal_hotplug_cleanup() would try to join a non-existent thread). */
13661370
libusb_exit(hid_hotplug_context.context);
1371+
hid_free_enumeration(hid_hotplug_context.devs);
1372+
hid_hotplug_context.devs = NULL;
13671373
free(hotplug_cb);
13681374
hid_hotplug_context.hotplug_cbs = NULL;
13691375
pthread_mutex_unlock(&hid_hotplug_context.mutex);

0 commit comments

Comments
 (0)