Commit 1dc24160a133abbaa6aae47eeb27beaf09d9da80
2019-07-17T16:47:13
Add linked list of opened HID devices to prevent accessing already freed devices in device removal callback that is sometimes called even after being unregistered
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180
diff --git a/src/hidapi/mac/hid.c b/src/hidapi/mac/hid.c
index d462d26..af34dbf 100644
--- a/src/hidapi/mac/hid.c
+++ b/src/hidapi/mac/hid.c
@@ -121,15 +121,16 @@ struct hid_device_ {
pthread_barrier_t barrier; /* Ensures correct startup sequence */
pthread_barrier_t shutdown_barrier; /* Ensures correct shutdown sequence */
int shutdown_thread;
-
- hid_device *next;
};
-/* Static list of all the devices open. This way when a device gets
- disconnected, its hid_device structure can be marked as disconnected
- from hid_device_removal_callback(). */
-static hid_device *device_list = NULL;
-static pthread_mutex_t device_list_mutex = PTHREAD_MUTEX_INITIALIZER;
+struct hid_device_list_node
+{
+ struct hid_device_ *dev;
+ struct hid_device_list_node *next;
+};
+
+static IOHIDManagerRef hid_mgr = 0x0;
+static hid_device_list_node *device_list = 0x0;
static hid_device *new_hid_device(void)
{
@@ -144,7 +145,6 @@ static hid_device *new_hid_device(void)
dev->input_report_buf = NULL;
dev->input_reports = NULL;
dev->shutdown_thread = 0;
- dev->next = NULL;
/* Thread objects */
pthread_mutex_init(&dev->mutex, NULL);
@@ -152,22 +152,6 @@ static hid_device *new_hid_device(void)
pthread_barrier_init(&dev->barrier, NULL, 2);
pthread_barrier_init(&dev->shutdown_barrier, NULL, 2);
- /* Add the new record to the device_list. */
- pthread_mutex_lock(&device_list_mutex);
- if (!device_list)
- device_list = dev;
- else {
- hid_device *d = device_list;
- while (d) {
- if (!d->next) {
- d->next = dev;
- break;
- }
- d = d->next;
- }
- }
- pthread_mutex_unlock(&device_list_mutex);
-
return dev;
}
@@ -193,6 +177,25 @@ static void free_hid_device(hid_device *dev)
if (dev->source)
CFRelease(dev->source);
free(dev->input_report_buf);
+
+ if (device_list) {
+ if (device_list->dev == dev) {
+ device_list = device_list->next;
+ }
+ else {
+ struct hid_device_list_node *node = device_list;
+ while (node) {
+ if (node->next && node->next->dev == dev) {
+ hid_device_list_node *new_next = node->next->next;
+ free(node->next);
+ node->next = new_next;
+ break;
+ }
+
+ node = node->next;
+ }
+ }
+ }
/* Clean up the thread objects */
pthread_barrier_destroy(&dev->shutdown_barrier);
@@ -200,31 +203,10 @@ static void free_hid_device(hid_device *dev)
pthread_cond_destroy(&dev->condition);
pthread_mutex_destroy(&dev->mutex);
- /* Remove it from the device list. */
- pthread_mutex_lock(&device_list_mutex);
- hid_device *d = device_list;
- if (d == dev) {
- device_list = d->next;
- }
- else {
- while (d) {
- if (d->next == dev) {
- d->next = d->next->next;
- break;
- }
-
- d = d->next;
- }
- }
- pthread_mutex_unlock(&device_list_mutex);
-
/* Free the structure itself. */
free(dev);
}
-static IOHIDManagerRef hid_mgr = 0x0;
-
-
#if 0
static void register_error(hid_device *device, const char *op)
{
@@ -588,20 +570,27 @@ hid_device * HID_API_EXPORT hid_open(unsigned short vendor_id, unsigned short pr
}
static void hid_device_removal_callback(void *context, IOReturn result,
- void *sender, IOHIDDeviceRef dev_ref)
+ void *sender)
{
/* Stop the Run Loop for this device. */
- pthread_mutex_lock(&device_list_mutex);
- hid_device *d = device_list;
- while (d) {
- if (d->device_handle == dev_ref) {
- d->disconnected = 1;
- CFRunLoopStop(d->run_loop);
+ hid_device *dev = (hid_device *)context;
+
+ // The device removal callback is sometimes called even after being
+ // unregistered, leading to a crash when trying to access fields in
+ // the already freed hid_device. We keep a linked list of all created
+ // hid_device's so that the one being removed can be checked against
+ // the list to see if it really hasn't been closed yet and needs to
+ // be dealt with here.
+ hid_device_list_node *node = device_list;
+ while (node) {
+ if (node->dev == dev) {
+ dev->disconnected = 1;
+ CFRunLoopStop(dev->run_loop);
+ break;
}
-
- d = d->next;
+
+ node = node->next;
}
- pthread_mutex_unlock(&device_list_mutex);
}
/* The Run Loop calls this function for each input report received.
@@ -777,8 +766,13 @@ hid_device * HID_API_EXPORT hid_open_path(const char *path, int bExclusive)
IOHIDDeviceRegisterInputReportCallback(
os_dev, dev->input_report_buf, dev->max_input_report_len,
&hid_report_callback, dev);
- IOHIDManagerRegisterDeviceRemovalCallback(hid_mgr, hid_device_removal_callback, NULL);
-
+ IOHIDDeviceRegisterRemovalCallback(dev->device_handle, hid_device_removal_callback, dev);
+
+ hid_device_list_node *node = (hid_device_list_node *)calloc(1, sizeof(hid_device_list_node));
+ node->dev = dev;
+ node->next = device_list;
+ device_list = node;
+
/* Start the read thread */
pthread_create(&dev->thread, NULL, read_thread, dev);
@@ -1048,7 +1042,7 @@ void HID_API_EXPORT hid_close(hid_device *dev)
IOHIDDeviceRegisterInputReportCallback(
dev->device_handle, dev->input_report_buf, dev->max_input_report_len,
NULL, dev);
- IOHIDManagerRegisterDeviceRemovalCallback(hid_mgr, NULL, dev);
+ IOHIDDeviceRegisterRemovalCallback(dev->device_handle, NULL, dev);
IOHIDDeviceUnscheduleFromRunLoop(dev->device_handle, dev->run_loop, dev->run_loop_mode);
IOHIDDeviceScheduleWithRunLoop(dev->device_handle, CFRunLoopGetMain(), kCFRunLoopDefaultMode);
}