windows: Obtain serial number from ioctl instead of instance ID

The instance ID is capitalized if the string descriptor contains lower case characters, and also uses a generated ID in place of the actual serial number if the serial number is a duplicate. Backport of 4ddd81b 6562dd9
kevinmehall · Nov 21, 2024 · 77b195c · 77b195c
1 parent 4bdc50b
commit 77b195c
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 36 deletions.
diff --git a/src/descriptors.rs b/src/descriptors.rs
@@ -28,6 +28,8 @@ pub(crate) const DESCRIPTOR_LEN_INTERFACE: u8 = 9;
 pub(crate) const DESCRIPTOR_TYPE_ENDPOINT: u8 = 0x05;
 pub(crate) const DESCRIPTOR_LEN_ENDPOINT: u8 = 7;
 
+pub(crate) const DESCRIPTOR_TYPE_STRING: u8 = 0x03;
+
 /// USB defined language IDs for string descriptors.
 ///
 /// In practice, different language IDs are not used,
@@ -549,6 +551,24 @@ pub(crate) fn parse_concatenated_config_descriptors(mut buf: &[u8]) -> impl Iter
     })
 }
 
+pub(crate) fn validate_string_descriptor(data: &[u8]) -> bool {
+    data.len() >= 2 && data[0] as usize == data.len() && data[1] == DESCRIPTOR_TYPE_STRING
+}
+
+pub(crate) fn decode_string_descriptor(data: &[u8]) -> Result<String, ()> {
+    if !validate_string_descriptor(data) {
+        return Err(());
+    }
+
+    Ok(char::decode_utf16(
+        data[2..]
+            .chunks_exact(2)
+            .map(|c| u16::from_le_bytes(c.try_into().unwrap())),
+    )
+    .map(|r| r.unwrap_or(char::REPLACEMENT_CHARACTER))
+    .collect::<String>())
+}
+
 /// Make public when fuzzing
 #[cfg(fuzzing)]
 pub fn fuzz_parse_concatenated_config_descriptors(buf: &[u8]) -> impl Iterator<Item = &[u8]> {

diff --git a/src/device.rs b/src/device.rs
@@ -3,7 +3,10 @@ use std::{io::ErrorKind, sync::Arc, time::Duration};
 use log::error;
 
 use crate::{
-    descriptors::{ActiveConfigurationError, Configuration, InterfaceAltSetting},
+    descriptors::{
+        decode_string_descriptor, validate_string_descriptor, ActiveConfigurationError,
+        Configuration, InterfaceAltSetting, DESCRIPTOR_TYPE_STRING,
+    },
     platform,
     transfer::{
         Control, ControlIn, ControlOut, EndpointType, Queue, RequestBuffer, TransferError,
@@ -12,9 +15,6 @@ use crate::{
     DeviceInfo, Error,
 };
 
-const STANDARD_REQUEST_GET_DESCRIPTOR: u8 = 0x06;
-const DESCRIPTOR_TYPE_STRING: u8 = 0x03;
-
 /// An opened USB device.
 ///
 /// Obtain a `Device` by calling [`DeviceInfo::open`]:
@@ -157,6 +157,7 @@ impl Device {
 
         #[cfg(not(target_os = "windows"))]
         {
+            const STANDARD_REQUEST_GET_DESCRIPTOR: u8 = 0x06;
             use crate::transfer::{ControlType, Recipient};
 
             let mut buf = vec![0; 4096];
@@ -187,7 +188,14 @@ impl Device {
         timeout: Duration,
     ) -> Result<impl Iterator<Item = u16>, Error> {
         let data = self.get_descriptor(DESCRIPTOR_TYPE_STRING, 0, 0, timeout)?;
-        validate_string_descriptor(&data)?;
+
+        if !validate_string_descriptor(&data) {
+            error!("String descriptor language list read {data:?}, not a valid string descriptor");
+            return Err(Error::new(
+                ErrorKind::InvalidData,
+                "string descriptor data was invalid",
+            ));
+        }
 
         //TODO: Use array_chunks once stable
         let mut iter = data.into_iter().skip(2);
@@ -218,15 +226,9 @@ impl Device {
             ));
         }
         let data = self.get_descriptor(DESCRIPTOR_TYPE_STRING, desc_index, language_id, timeout)?;
-        validate_string_descriptor(&data)?;
 
-        Ok(char::decode_utf16(
-            data[2..]
-                .chunks_exact(2)
-                .map(|c| u16::from_le_bytes(c.try_into().unwrap())),
-        )
-        .map(|r| r.unwrap_or(char::REPLACEMENT_CHARACTER))
-        .collect::<String>())
+        decode_string_descriptor(&data)
+            .map_err(|_| Error::new(ErrorKind::InvalidData, "string descriptor data was invalid"))
     }
 
     /// Reset the device, forcing it to re-enumerate.
@@ -343,17 +345,6 @@ impl Device {
     }
 }
 
-fn validate_string_descriptor(data: &[u8]) -> Result<(), Error> {
-    if data.len() < 2 || data[0] as usize != data.len() || data[1] != DESCRIPTOR_TYPE_STRING {
-        error!("String descriptor language list read {data:?}, not a valid string descriptor");
-        return Err(Error::new(
-            ErrorKind::InvalidData,
-            "string descriptor data was invalid",
-        ));
-    }
-    Ok(())
-}
-
 /// An opened interface of a USB device.
 ///
 /// Obtain an `Interface` with the [`Device::claim_interface`] method.

diff --git a/src/enumeration.rs b/src/enumeration.rs
@@ -209,12 +209,6 @@ impl DeviceInfo {
     }
 
     /// Serial number string, if available without device IO.
-    ///
-    /// ### Platform-specific notes
-    /// * On Windows, this comes from a case-insensitive instance ID and may
-    /// have been converted to upper case from the descriptor string. It is
-    /// recommended to use a [case-insensitive
-    /// comparison][str::eq_ignore_ascii_case] when matching a device.
     #[doc(alias = "iSerial")]
     pub fn serial_number(&self) -> Option<&str> {
         self.serial_number.as_deref()

diff --git a/src/platform/windows_winusb/enumeration.rs b/src/platform/windows_winusb/enumeration.rs
@@ -13,7 +13,10 @@ use windows_sys::Win32::Devices::{
     Usb::GUID_DEVINTERFACE_USB_DEVICE,
 };
 
-use crate::{DeviceInfo, Error, InterfaceInfo};
+use crate::{
+    descriptors::{decode_string_descriptor, language_id::US_ENGLISH, DESCRIPTOR_TYPE_STRING},
+    DeviceInfo, Error, InterfaceInfo,
+};
 
 use super::{
     cfgmgr32::{self, get_device_interface_property, DevInst},
@@ -47,9 +50,17 @@ pub fn probe_device(devinst: DevInst) -> Option<DeviceInfo> {
         .and_then(|s| s.into_string().ok());
 
     let serial_number = if info.device_desc.iSerialNumber != 0 {
-        (&instance_id)
-            .to_str()
-            .and_then(|s| s.rsplit_once("\\").map(|(_, s)| s.to_string()))
+        // Experimentally confirmed, the string descriptor is cached and this does
+        // not perform IO. However, the language ID list is not cached, so we
+        // have to assume 0x0409 (which will be right 99% of the time).
+        hub_port
+            .get_descriptor(
+                DESCRIPTOR_TYPE_STRING,
+                info.device_desc.iSerialNumber,
+                US_ENGLISH,
+            )
+            .ok()
+            .and_then(|data| decode_string_descriptor(&data).ok())
     } else {
         None
     };

diff --git a/src/platform/windows_winusb/hub.rs b/src/platform/windows_winusb/hub.rs
@@ -8,7 +8,7 @@ use std::{
     slice,
 };
 
-use log::{debug, warn};
+use log::debug;
 use windows_sys::Win32::{
     Devices::{
         Properties::DEVPKEY_Device_Address,
@@ -176,7 +176,7 @@ impl HubHandle {
                 Ok(vec)
             } else {
                 let err = GetLastError();
-                warn!("IOCTL_USB_GET_DESCRIPTOR_FROM_NODE_CONNECTION failed: type={descriptor_type} index={descriptor_index} error={err:?}");
+                debug!("IOCTL_USB_GET_DESCRIPTOR_FROM_NODE_CONNECTION failed: type={descriptor_type} index={descriptor_index} error={err:?}");
                 Err(match err {
                     ERROR_GEN_FAILURE => Error::new(
                         ErrorKind::Other,