From 96a1f3737584b6b3384d0d5e821e8b7fcae8f2cb Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Fri, 9 Aug 2019 08:59:27 -0700 Subject: darwin: fix bugs in the reenumeration of devices This commit fixes two bugs in device re-enumeration (enumeration after init): - The internal device object (cached device) was being re-used but the user-visible one was not. This would cause the user to get stale data when using the device afer reset. Now the (per libusb spec) the user-visible device object is re-used and updated. - If multiple libusb contexts were active then only the first context was handled correctly. Fixed by moving the cached device search out of process_new_device(). Signed-off-by: Nathan Hjelm --- libusb/os/darwin_usb.c | 134 ++++++++++++++++++++++++++++++++----------------- libusb/version_nano.h | 2 +- 2 files changed, 89 insertions(+), 47 deletions(-) diff --git a/libusb/os/darwin_usb.c b/libusb/os/darwin_usb.c index 43efb11..7fdf88f 100644 --- a/libusb/os/darwin_usb.c +++ b/libusb/os/darwin_usb.c @@ -101,7 +101,11 @@ static int darwin_reset_device(struct libusb_device_handle *dev_handle); static void darwin_async_io_callback (void *refcon, IOReturn result, void *arg0); static enum libusb_error darwin_scan_devices(struct libusb_context *ctx); -static enum libusb_error process_new_device (struct libusb_context *ctx, io_service_t service); +static enum libusb_error process_new_device (struct libusb_context *ctx, struct darwin_cached_device *cached_device, + UInt64 old_session_id); + +static enum libusb_error darwin_get_cached_device(io_service_t service, struct darwin_cached_device **cached_out, + UInt64 *old_session_id); #if defined(ENABLE_LOGGING) static const char *darwin_error_str (IOReturn result) { @@ -176,7 +180,10 @@ static void darwin_deref_cached_device(struct darwin_cached_device *cached_dev) if (0 == cached_dev->refcount) { list_del(&cached_dev->list); - (*(cached_dev->device))->Release(cached_dev->device); + if (cached_dev->device) { + (*(cached_dev->device))->Release(cached_dev->device); + cached_dev->device = NULL; + } free (cached_dev); } } @@ -331,15 +338,28 @@ static usb_device_t **darwin_device_from_service (io_service_t service) static void darwin_devices_attached (void *ptr, io_iterator_t add_devices) { UNUSED(ptr); + struct darwin_cached_device *cached_device; + UInt64 old_session_id; struct libusb_context *ctx; io_service_t service; + int ret; usbi_mutex_lock(&active_contexts_lock); while ((service = IOIteratorNext(add_devices))) { + ret = darwin_get_cached_device (service, &cached_device, &old_session_id); + if (ret < 0 || !cached_device->can_enumerate) { + continue; + } + /* add this device to each active context's device list */ list_for_each_entry(ctx, &active_contexts_list, list, struct libusb_context) { - process_new_device (ctx, service); + process_new_device (ctx, cached_device, old_session_id); + } + + if (cached_device->in_reenumerate) { + usbi_dbg ("cached device in reset state. reset complete..."); + cached_device->in_reenumerate = false; } IOObjectRelease(service); @@ -361,6 +381,8 @@ static void darwin_devices_detached (void *ptr, io_iterator_t rem_devices) { usbi_mutex_lock(&active_contexts_lock); while ((device = IOIteratorNext (rem_devices)) != 0) { + bool is_reenumerating = false; + /* get the location from the i/o registry */ ret = get_ioregistry_value_number (device, CFSTR("sessionID"), kCFNumberSInt64Type, &session); IOObjectRelease (device); @@ -376,14 +398,24 @@ static void darwin_devices_detached (void *ptr, io_iterator_t rem_devices) { /* device is re-enumerating. do not dereference the device at this time. libusb_reset_device() * will deref if needed. */ usbi_dbg ("detected device detatched due to re-enumeration"); + + /* the device object is no longer usable so go ahead and release it */ + if (old_device->device) { + (*(old_device->device))->Release(old_device->device); + old_device->device = NULL; + } + + is_reenumerating = true; } else { darwin_deref_cached_device (old_device); } + break; } } + usbi_mutex_unlock(&darwin_cached_devices_lock); - if (old_device->in_reenumerate) { + if (is_reenumerating) { continue; } @@ -747,7 +779,7 @@ static IOReturn darwin_request_descriptor (usb_device_t **device, UInt8 desc, UI return (*device)->DeviceRequestTO (device, &req); } -static enum libusb_error darwin_cache_device_descriptor (struct libusb_context *ctx, struct darwin_cached_device *dev) { +static enum libusb_error darwin_cache_device_descriptor (struct darwin_cached_device *dev) { usb_device_t **device = dev->device; int retries = 1; long delay = 30000; // microseconds @@ -844,7 +876,7 @@ static enum libusb_error darwin_cache_device_descriptor (struct libusb_context * usbi_dbg ("could not retrieve device descriptor %.4x:%.4x: %s (%x). skipping device", idVendor, idProduct, darwin_error_str (ret), ret); else - usbi_warn (ctx, "could not retrieve device descriptor %.4x:%.4x: %s (%x). skipping device", + usbi_warn (NULL, "could not retrieve device descriptor %.4x:%.4x: %s (%x). skipping device", idVendor, idProduct, darwin_error_str (ret), ret); return darwin_to_libusb (ret); } @@ -852,7 +884,7 @@ static enum libusb_error darwin_cache_device_descriptor (struct libusb_context * /* catch buggy hubs (which appear to be virtual). Apple's own USB prober has problems with these devices. */ if (libusb_le16_to_cpu (dev->dev_descriptor.idProduct) != idProduct) { /* not a valid device */ - usbi_warn (ctx, "idProduct from iokit (%04x) does not match idProduct in descriptor (%04x). skipping device", + usbi_warn (NULL, "idProduct from iokit (%04x) does not match idProduct in descriptor (%04x). skipping device", idProduct, libusb_le16_to_cpu (dev->dev_descriptor.idProduct)); return LIBUSB_ERROR_NO_DEVICE; } @@ -914,15 +946,18 @@ static bool get_device_parent_sessionID(io_service_t service, UInt64 *parent_ses return false; } -static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io_service_t service, - struct darwin_cached_device **cached_out) { +static enum libusb_error darwin_get_cached_device(io_service_t service, struct darwin_cached_device **cached_out, + UInt64 *old_session_id) { struct darwin_cached_device *new_device; UInt64 sessionID = 0, parent_sessionID = 0; UInt32 locationID = 0; enum libusb_error ret = LIBUSB_SUCCESS; usb_device_t **device; UInt8 port = 0; - bool reuse_device = false; + + /* assuming sessionID != 0 normally (never seen it be 0) */ + *old_session_id = 0; + *cached_out = NULL; /* get some info from the io registry */ (void) get_ioregistry_value_number (service, CFSTR("sessionID"), kCFNumberSInt64Type, &sessionID); @@ -939,15 +974,12 @@ static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io usbi_mutex_lock(&darwin_cached_devices_lock); do { - *cached_out = NULL; - list_for_each_entry(new_device, &darwin_cached_devices, list, struct darwin_cached_device) { usbi_dbg("matching sessionID/locationID 0x%" PRIx64 "/0x%x against cached device with sessionID/locationID 0x%" PRIx64 "/0x%x", sessionID, locationID, new_device->session, new_device->location); if (new_device->location == locationID && new_device->in_reenumerate) { usbi_dbg ("found cached device with matching location that is being re-enumerated"); - new_device->session = sessionID; - reuse_device = true; + *old_session_id = new_device->session; break; } @@ -969,7 +1001,7 @@ static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io break; } - if (!reuse_device) { + if (!(*old_session_id)) { new_device = calloc (1, sizeof (*new_device)); if (!new_device) { ret = LIBUSB_ERROR_NO_MEM; @@ -984,16 +1016,20 @@ static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io /* keep a reference to this device */ darwin_ref_cached_device(new_device); - new_device->session = sessionID; (*device)->GetLocationID (device, &new_device->location); new_device->port = port; new_device->parent_session = parent_sessionID; } + /* keep track of devices regardless of if we successfully enumerate them to + prevent them from being enumerated multiple times */ + *cached_out = new_device; + + new_device->session = sessionID; new_device->device = device; /* cache the device descriptor */ - ret = darwin_cache_device_descriptor(ctx, new_device); + ret = darwin_cache_device_descriptor(new_device); if (ret) break; @@ -1006,55 +1042,56 @@ static enum libusb_error darwin_get_cached_device(struct libusb_context *ctx, io usbi_mutex_unlock(&darwin_cached_devices_lock); - /* keep track of devices regardless of if we successfully enumerate them to - prevent them from being enumerated multiple times */ - - *cached_out = new_device; - return ret; } -static enum libusb_error process_new_device (struct libusb_context *ctx, io_service_t service) { +static enum libusb_error process_new_device (struct libusb_context *ctx, struct darwin_cached_device *cached_device, + UInt64 old_session_id) { struct darwin_device_priv *priv; struct libusb_device *dev = NULL; - struct darwin_cached_device *cached_device; UInt8 devSpeed; enum libusb_error ret = LIBUSB_SUCCESS; do { - ret = darwin_get_cached_device (ctx, service, &cached_device); - if (ret < 0 || !cached_device->can_enumerate) { - return ret; - } - /* check current active configuration (and cache the first configuration value-- which may be used by claim_interface) */ ret = darwin_check_configuration (ctx, cached_device); if (ret) break; - usbi_dbg ("allocating new device in context %p for with session 0x%" PRIx64, - ctx, cached_device->session); + if (0 != old_session_id) { + usbi_dbg ("re-using existing device from context %p for with session 0x%" PRIx64 " new session 0x%" PRIx64, + ctx, old_session_id, cached_device->session); + /* save the libusb device before the session id is updated */ + dev = usbi_get_device_by_session_id (ctx, (unsigned long) old_session_id); + } - dev = usbi_alloc_device(ctx, (unsigned long) cached_device->session); if (!dev) { - return LIBUSB_ERROR_NO_MEM; - } + usbi_dbg ("allocating new device in context %p for with session 0x%" PRIx64, + ctx, cached_device->session); - priv = (struct darwin_device_priv *)dev->os_priv; + dev = usbi_alloc_device(ctx, (unsigned long) cached_device->session); + if (!dev) { + return LIBUSB_ERROR_NO_MEM; + } - priv->dev = cached_device; - darwin_ref_cached_device (priv->dev); + priv = (struct darwin_device_priv *)dev->os_priv; + + priv->dev = cached_device; + darwin_ref_cached_device (priv->dev); + dev->port_number = cached_device->port; + dev->bus_number = cached_device->location >> 24; + assert(cached_device->address <= UINT8_MAX); + dev->device_address = (uint8_t)cached_device->address; + } else { + priv = (struct darwin_device_priv *)dev->os_priv; + } if (cached_device->parent_session > 0) { dev->parent_dev = usbi_get_device_by_session_id (ctx, (unsigned long) cached_device->parent_session); } else { dev->parent_dev = NULL; } - dev->port_number = cached_device->port; - dev->bus_number = cached_device->location >> 24; - assert(cached_device->address <= UINT8_MAX); - dev->device_address = (uint8_t)cached_device->address; (*(priv->dev->device))->GetDeviceSpeed (priv->dev->device, &devSpeed); @@ -1081,10 +1118,7 @@ static enum libusb_error process_new_device (struct libusb_context *ctx, io_serv } while (0); - if (cached_device->in_reenumerate) { - usbi_dbg ("cached device in reset state. reset complete..."); - cached_device->in_reenumerate = false; - } else if (0 == ret) { + if (!cached_device->in_reenumerate && 0 == ret) { usbi_connect_device (dev); } else { libusb_unref_device (dev); @@ -1094,16 +1128,24 @@ static enum libusb_error process_new_device (struct libusb_context *ctx, io_serv } static enum libusb_error darwin_scan_devices(struct libusb_context *ctx) { + struct darwin_cached_device *cached_device; + UInt64 old_session_id; io_iterator_t deviceIterator; io_service_t service; IOReturn kresult; + int ret; kresult = usb_setup_device_iterator (&deviceIterator, 0); if (kresult != kIOReturnSuccess) return darwin_to_libusb (kresult); while ((service = IOIteratorNext (deviceIterator))) { - (void) process_new_device (ctx, service); + ret = darwin_get_cached_device (service, &cached_device, &old_session_id); + if (ret < 0 || !cached_device->can_enumerate) { + continue; + } + + (void) process_new_device (ctx, cached_device, old_session_id); IOObjectRelease(service); } diff --git a/libusb/version_nano.h b/libusb/version_nano.h index d7f210e..ba8b13c 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11378 +#define LIBUSB_NANO 11379 -- cgit v1.2.1