darwin: fix race condition on device capture

If two threads try to capture at the same time, the result can be
inconsistent. For example, a process can have two different contexts to
libusb and both call `libusb_reset_device`.

Author: osy

libusb/os/darwin_usb.c

@@ -90,6 +90,7 @@ static int darwin_reenumerate_device(struct libusb_device_handle *dev_handle, bo
 static int darwin_clear_halt(struct libusb_device_handle *dev_handle, unsigned char endpoint);
 static int darwin_reset_device(struct libusb_device_handle *dev_handle);
 static int darwin_detach_kernel_driver (struct libusb_device_handle *dev_handle, uint8_t interface);
+static int _darwin_detach_kernel_driver_locked (struct libusb_device_handle *dev_handle, uint8_t interface);
 static void darwin_async_io_callback (void *refcon, IOReturn result, void *arg0);
 
 static enum libusb_error darwin_scan_devices(struct libusb_context *ctx);
@@ -457,6 +458,8 @@ static void darwin_deref_cached_device(struct darwin_cached_device *cached_dev)
       cached_dev->device = NULL;
     }
     IOObjectRelease (cached_dev->service);
+    usbi_mutex_destroy (&cached_dev->capture_mutex);
+    usbi_mutex_destroy (&cached_dev->open_mutex);
     free (cached_dev);
   }
 }
@@ -1358,6 +1361,10 @@ static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io
       (*device)->GetLocationID (device, &new_device->location);
       new_device->port = port;
       new_device->parent_session = parent_sessionID;
+
+      /* initialize locks */
+      usbi_mutex_init (&new_device->capture_mutex);
+      usbi_mutex_init (&new_device->open_mutex);
     } else {
       /* release the ref to old device's service */
       IOObjectRelease (new_device->service);
@@ -1519,7 +1526,8 @@ static enum libusb_error darwin_scan_devices(struct libusb_context *ctx) {
   return LIBUSB_SUCCESS;
 }
 
-static int darwin_open (struct libusb_device_handle *dev_handle) {
+/* call holding open_mutex lock */
+static int _darwin_open_locked (struct libusb_device_handle *dev_handle) {
   struct darwin_device_handle_priv *priv = usbi_get_device_handle_priv(dev_handle);
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   IOReturn kresult;
@@ -1569,7 +1577,18 @@ static int darwin_open (struct libusb_device_handle *dev_handle) {
   return 0;
 }
 
-static void darwin_close (struct libusb_device_handle *dev_handle) {
+static int darwin_open (struct libusb_device_handle *dev_handle) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+  enum libusb_error ret;
+
+  usbi_mutex_lock (&dpriv->open_mutex);
+  ret = _darwin_open_locked (dev_handle);
+  usbi_mutex_unlock (&dpriv->open_mutex);
+  return ret;
+}
+
+/* call holding open_mutex lock */
+static void _darwin_close_locked (struct libusb_device_handle *dev_handle) {
   struct darwin_device_handle_priv *priv = usbi_get_device_handle_priv(dev_handle);
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   IOReturn kresult;
@@ -1613,6 +1632,14 @@ static void darwin_close (struct libusb_device_handle *dev_handle) {
   }
 }
 
+static void darwin_close (struct libusb_device_handle *dev_handle) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+
+  usbi_mutex_lock (&dpriv->open_mutex);
+  _darwin_close_locked (dev_handle);
+  usbi_mutex_unlock (&dpriv->open_mutex);
+}
+
 static int darwin_get_configuration(struct libusb_device_handle *dev_handle, uint8_t *config) {
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
 
@@ -2011,11 +2038,15 @@ static int darwin_restore_state (struct libusb_device_handle *dev_handle, uint8_
                                  unsigned long claimed_interfaces) {
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   struct darwin_device_handle_priv *priv = usbi_get_device_handle_priv(dev_handle);
-  int open_count = dpriv->open_count;
+  int open_count;
   int ret;
 
   struct libusb_context *ctx = HANDLE_CTX (dev_handle);
 
+  usbi_mutex_lock (&dpriv->open_mutex);
+
+  open_count = dpriv->open_count;
+
   /* clear claimed interfaces temporarily */
   dev_handle->claimed_interfaces = 0;
 
@@ -2024,11 +2055,14 @@ static int darwin_restore_state (struct libusb_device_handle *dev_handle, uint8_
   dpriv->open_count = 1;
 
   /* clean up open interfaces */
-  (void) darwin_close (dev_handle);
+  (void) _darwin_close_locked (dev_handle);
 
   /* re-open the device */
-  ret = darwin_open (dev_handle);
+  ret = _darwin_open_locked (dev_handle);
   dpriv->open_count = open_count;
+
+  usbi_mutex_unlock (&dpriv->open_mutex);
+
   if (LIBUSB_SUCCESS != ret) {
     /* could not restore configuration */
     return LIBUSB_ERROR_NOT_FOUND;
@@ -2069,6 +2103,7 @@ static int darwin_restore_state (struct libusb_device_handle *dev_handle, uint8_
   return LIBUSB_SUCCESS;
 }
 
+/* call holding capture_mutex lock */
 static int darwin_reenumerate_device (struct libusb_device_handle *dev_handle, bool capture) {
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   unsigned long claimed_interfaces = dev_handle->claimed_interfaces;
@@ -2168,7 +2203,8 @@ static int darwin_reenumerate_device (struct libusb_device_handle *dev_handle, b
   return darwin_restore_state (dev_handle, active_config, claimed_interfaces);
 }
 
-static int darwin_reset_device (struct libusb_device_handle *dev_handle) {
+/* call holding capture_mutex lock */
+static int _darwin_reset_device_locked (struct libusb_device_handle *dev_handle) {
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   IOReturn kresult;
   enum libusb_error ret;
@@ -2194,7 +2230,7 @@ static int darwin_reset_device (struct libusb_device_handle *dev_handle) {
     /* reset capture count */
     dpriv->capture_count = 0;
     /* attempt to detach kernel driver again as it is now re-attached */
-    ret = darwin_detach_kernel_driver (dev_handle, 0);
+    ret = _darwin_detach_kernel_driver_locked (dev_handle, 0);
     if (ret != LIBUSB_SUCCESS) {
       return ret;
     }
@@ -2207,6 +2243,16 @@ static int darwin_reset_device (struct libusb_device_handle *dev_handle) {
   return ret;
 }
 
+static int darwin_reset_device (struct libusb_device_handle *dev_handle) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+  enum libusb_error ret;
+
+  usbi_mutex_lock (&dpriv->capture_mutex);
+  ret = _darwin_reset_device_locked (dev_handle);
+  usbi_mutex_unlock (&dpriv->capture_mutex);
+  return ret;
+}
+
 static io_service_t usb_find_interface_matching_location (const io_name_t class_name, UInt8 interface_number, UInt32 location) {
   CFMutableDictionaryRef matchingDict = IOServiceMatching (class_name);
   CFMutableDictionaryRef propertyMatchDict = CFDictionaryCreateMutable (kCFAllocatorDefault, 0,
@@ -2822,7 +2868,8 @@ static int darwin_reload_device (struct libusb_device_handle *dev_handle) {
 
 /* On macOS, we capture an entire device at once, not individual interfaces. */
 
-static int darwin_detach_kernel_driver (struct libusb_device_handle *dev_handle, uint8_t interface) {
+/* call holding capture_mutex lock */
+static int _darwin_detach_kernel_driver_locked (struct libusb_device_handle *dev_handle, uint8_t interface) {
   UNUSED(interface);
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
   IOReturn kresult;
@@ -2867,8 +2914,18 @@ static int darwin_detach_kernel_driver (struct libusb_device_handle *dev_handle,
   return LIBUSB_SUCCESS;
 }
 
+static int darwin_detach_kernel_driver (struct libusb_device_handle *dev_handle, uint8_t interface) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+  enum libusb_error ret;
+
+  usbi_mutex_lock (&dpriv->capture_mutex);
+  ret = _darwin_detach_kernel_driver_locked (dev_handle, interface);
+  usbi_mutex_unlock (&dpriv->capture_mutex);
+  return ret;
+}
 
-static int darwin_attach_kernel_driver (struct libusb_device_handle *dev_handle, uint8_t interface) {
+/* call holding capture_mutex lock */
+static int _darwin_attach_kernel_driver_locked (struct libusb_device_handle *dev_handle, uint8_t interface) {
   UNUSED(interface);
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
 
@@ -2887,6 +2944,16 @@ static int darwin_attach_kernel_driver (struct libusb_device_handle *dev_handle,
   return darwin_reenumerate_device (dev_handle, false);
 }
 
+static int darwin_attach_kernel_driver (struct libusb_device_handle *dev_handle, uint8_t interface) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+  enum libusb_error ret;
+
+  usbi_mutex_lock (&dpriv->capture_mutex);
+  ret = _darwin_attach_kernel_driver_locked (dev_handle, interface);
+  usbi_mutex_unlock (&dpriv->capture_mutex);
+  return ret;
+}
+
 static int darwin_capture_claim_interface(struct libusb_device_handle *dev_handle, uint8_t iface) {
   enum libusb_error ret;
   if (dev_handle->auto_detach_kernel_driver && darwin_kernel_driver_active(dev_handle, iface)) {
@@ -2899,7 +2966,8 @@ static int darwin_capture_claim_interface(struct libusb_device_handle *dev_handl
   return darwin_claim_interface (dev_handle, iface);
 }
 
-static int darwin_capture_release_interface(struct libusb_device_handle *dev_handle, uint8_t iface) {
+/* call holding capture_mutex lock */
+static int _darwin_capture_release_interface_locked(struct libusb_device_handle *dev_handle, uint8_t iface) {
   enum libusb_error ret;
   struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
 
@@ -2909,7 +2977,7 @@ static int darwin_capture_release_interface(struct libusb_device_handle *dev_han
   }
 
   if (dev_handle->auto_detach_kernel_driver && dpriv->capture_count > 0) {
-    ret = darwin_attach_kernel_driver (dev_handle, iface);
+    ret = _darwin_attach_kernel_driver_locked (dev_handle, iface);
     if (LIBUSB_SUCCESS != ret) {
       usbi_info (HANDLE_CTX (dev_handle), "on attempt to reattach the kernel driver got ret=%d", ret);
     }
@@ -2919,6 +2987,16 @@ static int darwin_capture_release_interface(struct libusb_device_handle *dev_han
   return LIBUSB_SUCCESS;
 }
 
+static int darwin_capture_release_interface (struct libusb_device_handle *dev_handle, uint8_t iface) {
+  struct darwin_cached_device *dpriv = DARWIN_CACHED_DEVICE(dev_handle->dev);
+  enum libusb_error ret;
+
+  usbi_mutex_lock (&dpriv->capture_mutex);
+  ret = _darwin_capture_release_interface_locked (dev_handle, iface);
+  usbi_mutex_unlock (&dpriv->capture_mutex);
+  return ret;
+}
+
 #endif
 
 const struct usbi_os_backend usbi_backend = {

libusb/os/darwin_usb.h

@@ -113,12 +113,14 @@ struct darwin_cached_device {
   char                  sys_path[21];
   usb_device_t          device;
   io_service_t          service;
-  int                   open_count;
+  usbi_mutex_t          open_mutex;
+  int                   open_count; // GUARDED_BY(open_mutex)
   UInt8                 first_config, active_config, port;
   int                   can_enumerate;
   int                   refcount;
   bool                  in_reenumerate;
-  int                   capture_count;
+  usbi_mutex_t          capture_mutex;
+  int                   capture_count; // GUARDED_BY(capture_mutex)
 };
 
 struct darwin_device_priv {