diff options
author | Sylvain Fasel <sylvain@sonatique.net> | 2023-04-15 17:36:43 +0200 |
---|---|---|
committer | Tormod Volden <debian.tormod@gmail.com> | 2023-11-05 14:46:00 +0100 |
commit | 4b732d94220c41f5bc23c572a5a5157de7ff0177 (patch) | |
tree | bbb424c3be7616676b11392f09912e647c37524c | |
parent | c875f158b58dfd0cafc7c7319c7971b9620e4f36 (diff) | |
download | libusb-4b732d94220c41f5bc23c572a5a5157de7ff0177.tar.gz |
windows: No longer wait for device to get an active configuration
This commit essentially reverts commit eb164d75 (from 2018, pre 1.0.23).
Commit eb164d75 was made with the assumption that when a device is not
configured (i.e. it returns CurrentConfigurationValue == 0 during
init_device), then it is only a temporary condition caused by Windows
already listing the device but not having completed its setup yet. The
commit added a delay of up to 1 second (20 x 50ms) for re-obtaining
information and checking until CurrentConfigurationValue would
eventually change to a value > 0. The commit also forced libusb internal
structure to contain 1 as the active configuration even if the wait
failed to retrieve an active configuration, creating a discrepancy in
between the value seen from libusb and actual value managed by Windows.
There is a problem with the above because devices could be in the
unconfigured state for permanent reasons. For instance when a device is
disabled by Windows (either manually using Device Manager or
automatically because of a resource conflict). In this case the device
is listed with CurrentConfigurationValue == 0. Therefore init_device
will add a 1 second delay for each disabled device, at every device
enumeration.
A device which is not configured has no active interface nor available
endpoint. Therefore it cannot be used (except for standard request on
endpoint 0) and therefore shall not be returned by libusb as an active
device. It is the responsibility of the OS to bring the device to a
configured state. When libusb returns active configuration == 1 to a
user, the behavior of actually using this device is not defined.
The potential gain of being able to wait for Windows actually completing
device setup is not in par with the delay caused by the waiting which
has high-level impact (for instance in GUI listing devices) for standard
scenarios.
The initial problem addressed by commit eb164d75 is simply a race
condition occurring (rarely) when libusb enumeration happens exactly at
the instant when Windows is not yet done loading the driver etc. for an
enabled device. Calling init_device slightly earlier would have return
no result for this device, and slightly later would have return a fully
active device. In the first case a new enumeration would be necessary
anyway. Given hot-plug is not supported under Windows it is a good
practice to re-enumerate devices to detect newly plugged ones anyway, so
the race condition could be easily solved by simply enumerating
regularly.
To summarize: This commit reverts to handling the described race
condition by ignoring any non-configured device (not returning them from
get_device_list), instead of waiting for them to maybe get an active
configuration at a later time. The benefit is to avoid introducing
arbitrary large delay in common scenarios where some devices are
disabled at the OS level.
Moreover this commit reverts assigning a wrong value to
priv->active_config, and returning such device from get_device_list,
preventing the risk of later misuse of not configured devices.
Note: Disabled devices might not necessarily be listed under USB devices
in Windows Device Manager, but can be other system peripherals, one
example being Bluetooth chips with HCI USB interfaces.
Closes #1270
-rw-r--r-- | libusb/os/windows_winusb.c | 73 | ||||
-rw-r--r-- | libusb/version_nano.h | 2 |
2 files changed, 29 insertions, 46 deletions
diff --git a/libusb/os/windows_winusb.c b/libusb/os/windows_winusb.c index 5344958..74f35fd 100644 --- a/libusb/os/windows_winusb.c +++ b/libusb/os/windows_winusb.c @@ -1074,7 +1074,6 @@ static int init_device(struct libusb_device *dev, struct libusb_device *parent_d DWORD size; uint8_t bus_number, depth; int r; - int ginfotimeout; priv = usbi_get_device_priv(dev); @@ -1135,61 +1134,45 @@ static int init_device(struct libusb_device *dev, struct libusb_device *parent_d conn_info.ConnectionIndex = (ULONG)port_number; // coverity[tainted_data_argument] - ginfotimeout = 20; - do { - if (!DeviceIoControl(hub_handle, IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX, &conn_info, sizeof(conn_info), - &conn_info, sizeof(conn_info), &size, NULL)) { - usbi_warn(ctx, "could not get node connection information for device '%s': %s", - priv->dev_id, windows_error_str(0)); - CloseHandle(hub_handle); - return LIBUSB_ERROR_NO_DEVICE; - } - - if (conn_info.ConnectionStatus == NoDeviceConnected) { - usbi_err(ctx, "device '%s' is no longer connected!", priv->dev_id); - CloseHandle(hub_handle); - return LIBUSB_ERROR_NO_DEVICE; - } - if ((conn_info.DeviceDescriptor.bLength != LIBUSB_DT_DEVICE_SIZE) - || (conn_info.DeviceDescriptor.bDescriptorType != LIBUSB_DT_DEVICE)) { - SleepEx(50, TRUE); - continue; - } + if (!DeviceIoControl(hub_handle, IOCTL_USB_GET_NODE_CONNECTION_INFORMATION_EX, &conn_info, sizeof(conn_info), + &conn_info, sizeof(conn_info), &size, NULL)) { + usbi_warn(ctx, "could not get node connection information for device '%s': %s", + priv->dev_id, windows_error_str(0)); + CloseHandle(hub_handle); + return LIBUSB_ERROR_NO_DEVICE; + } - static_assert(sizeof(dev->device_descriptor) == sizeof(conn_info.DeviceDescriptor), - "mismatch between libusb and OS device descriptor sizes"); - memcpy(&dev->device_descriptor, &conn_info.DeviceDescriptor, LIBUSB_DT_DEVICE_SIZE); - usbi_localize_device_descriptor(&dev->device_descriptor); - - priv->active_config = conn_info.CurrentConfigurationValue; - if (priv->active_config == 0) { - usbi_dbg(ctx, "0x%x:0x%x found %u configurations (not configured)", - dev->device_descriptor.idVendor, - dev->device_descriptor.idProduct, - dev->device_descriptor.bNumConfigurations); - SleepEx(50, TRUE); - } - } while (priv->active_config == 0 && --ginfotimeout >= 0); + if (conn_info.ConnectionStatus == NoDeviceConnected) { + usbi_err(ctx, "device '%s' is no longer connected!", priv->dev_id); + CloseHandle(hub_handle); + return LIBUSB_ERROR_NO_DEVICE; + } if ((conn_info.DeviceDescriptor.bLength != LIBUSB_DT_DEVICE_SIZE) - || (conn_info.DeviceDescriptor.bDescriptorType != LIBUSB_DT_DEVICE)) { + || (conn_info.DeviceDescriptor.bDescriptorType != LIBUSB_DT_DEVICE)) { usbi_err(ctx, "device '%s' has invalid descriptor!", priv->dev_id); CloseHandle(hub_handle); return LIBUSB_ERROR_OTHER; } - if (priv->active_config == 0) { - usbi_info(ctx, "0x%x:0x%x found %u configurations but device isn't configured, " - "forcing current configuration to 1", - dev->device_descriptor.idVendor, - dev->device_descriptor.idProduct, - dev->device_descriptor.bNumConfigurations); - priv->active_config = 1; - } else { - usbi_dbg(ctx, "found %u configurations (current config: %u)", dev->device_descriptor.bNumConfigurations, priv->active_config); + static_assert(sizeof(dev->device_descriptor) == sizeof(conn_info.DeviceDescriptor), + "mismatch between libusb and OS device descriptor sizes"); + memcpy(&dev->device_descriptor, &conn_info.DeviceDescriptor, LIBUSB_DT_DEVICE_SIZE); + usbi_localize_device_descriptor(&dev->device_descriptor); + + if (conn_info.CurrentConfigurationValue == 0) { + usbi_dbg(ctx, "found %u configurations for device '%s' but device is not configured (i.e. current config: 0), ignoring it", + dev->device_descriptor.bNumConfigurations, + priv->dev_id); + CloseHandle(hub_handle); + return LIBUSB_ERROR_OTHER; } + priv->active_config = conn_info.CurrentConfigurationValue; + usbi_dbg(ctx, "found %u configurations (current config: %u) for device '%s'", + dev->device_descriptor.bNumConfigurations, priv->active_config, priv->dev_id); + // Cache as many config descriptors as we can cache_config_descriptors(dev, hub_handle); diff --git a/libusb/version_nano.h b/libusb/version_nano.h index 3aab854..3b7bb88 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 11810 +#define LIBUSB_NANO 11811 |