aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSylvain Fasel <sylvain@sonatique.net>2023-04-15 17:36:43 +0200
committerTormod Volden <debian.tormod@gmail.com>2023-11-05 14:46:00 +0100
commit4b732d94220c41f5bc23c572a5a5157de7ff0177 (patch)
treebbb424c3be7616676b11392f09912e647c37524c
parentc875f158b58dfd0cafc7c7319c7971b9620e4f36 (diff)
downloadlibusb-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.c73
-rw-r--r--libusb/version_nano.h2
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