From 63809b97be2d672791ba24b98f02013938ca05ca Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sat, 2 Mar 2024 21:21:20 -0700 Subject: [PATCH] Document and warn on case where WinUSB changes the passed control transfer index field to the interface number --- src/device.rs | 32 +++++++++++++++++++++---- src/platform/windows_winusb/device.rs | 11 +++++++-- src/platform/windows_winusb/transfer.rs | 14 +++++++++-- 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/device.rs b/src/device.rs index 2e0bb0c..e55ea88 100644 --- a/src/device.rs +++ b/src/device.rs @@ -346,8 +346,13 @@ impl Interface { /// /// ### Platform-specific notes /// - /// * On Linux, this takes a device-wide lock, so if you have multiple threads, you - /// are better off using the async methods. + /// * On Linux, this takes a device-wide lock, so if you have multiple + /// threads, you are better off using the async methods. + /// * On Windows, if the `recipient` is `Interface`, the WinUSB driver sends + /// the interface number in the least significant byte of `index`, + /// overriding any value passed. A warning is logged if the passed `index` + /// least significant byte differs from the interface number, and this may + /// become an error in the future. pub fn control_in_blocking( &self, control: Control, @@ -361,8 +366,13 @@ impl Interface { /// /// ### Platform-specific notes /// - /// * On Linux, this takes a device-wide lock, so if you have multiple threads, you - /// are better off using the async methods. + /// * On Linux, this takes a device-wide lock, so if you have multiple + /// threads, you are better off using the async methods. + /// * On Windows, if the `recipient` is `Interface`, the WinUSB driver sends + /// the interface number in the least significant byte of `index`, + /// overriding any value passed. A warning is logged if the passed `index` + /// least significant byte differs from the interface number, and this may + /// become an error in the future. pub fn control_out_blocking( &self, control: Control, @@ -394,6 +404,13 @@ impl Interface { /// })).into_result()?; /// # Ok(()) } /// ``` + /// + /// ### Platform-specific notes + /// * On Windows, if the `recipient` is `Interface`, the WinUSB driver sends + /// the interface number in the least significant byte of `index`, + /// overriding any value passed. A warning is logged if the passed `index` + /// least significant byte differs from the interface number, and this may + /// become an error in the future. pub fn control_in(&self, data: ControlIn) -> TransferFuture { let mut t = self.backend.make_transfer(0, EndpointType::Control); t.submit::(data); @@ -422,6 +439,13 @@ impl Interface { /// })).into_result()?; /// # Ok(()) } /// ``` + /// + /// ### Platform-specific notes + /// * On Windows, if the `recipient` is `Interface`, the WinUSB driver sends + /// the interface number in the least significant byte of `index`, + /// overriding any value passed. A warning is logged if the passed `index` + /// least significant byte differs from the interface number, and this may + /// become an error in the future. pub fn control_out(&self, data: ControlOut) -> TransferFuture { let mut t = self.backend.make_transfer(0, EndpointType::Control); t.submit::(data); diff --git a/src/platform/windows_winusb/device.rs b/src/platform/windows_winusb/device.rs index 0757874..fedf5de 100644 --- a/src/platform/windows_winusb/device.rs +++ b/src/platform/windows_winusb/device.rs @@ -8,7 +8,7 @@ use std::{ time::Duration, }; -use log::{debug, error, info}; +use log::{debug, error, info, warn}; use windows_sys::Win32::{ Devices::Usb::{ WinUsb_ControlTransfer, WinUsb_Free, WinUsb_Initialize, WinUsb_SetCurrentAlternateSetting, @@ -19,7 +19,7 @@ use windows_sys::Win32::{ use crate::{ descriptors::{validate_config_descriptor, DESCRIPTOR_TYPE_CONFIGURATION}, - transfer::{Control, Direction, EndpointType, TransferError, TransferHandle}, + transfer::{Control, Direction, EndpointType, Recipient, TransferError, TransferHandle}, DeviceInfo, Error, }; @@ -125,6 +125,7 @@ impl WindowsDevice { Ok(Arc::new(WindowsInterface { handle, + interface, winusb_handle, })) } @@ -139,6 +140,7 @@ impl WindowsDevice { pub(crate) struct WindowsInterface { pub(crate) handle: OwnedHandle, + pub(crate) interface: u8, pub(crate) winusb_handle: WINUSB_INTERFACE_HANDLE, } @@ -161,6 +163,11 @@ impl WindowsInterface { timeout: Duration, ) -> Result { info!("Blocking control {direction:?}, {len} bytes"); + + if control.recipient == Recipient::Interface && control.index as u8 != self.interface { + warn!("WinUSB sends interface number instead of passed `index` when performing a control transfer with `Recipient::Interface`"); + } + let timeout_ms = timeout.as_millis().min(u32::MAX as u128) as u32; let r = WinUsb_SetPipePolicy( self.winusb_handle, diff --git a/src/platform/windows_winusb/transfer.rs b/src/platform/windows_winusb/transfer.rs index a7b73f7..fe39eda 100644 --- a/src/platform/windows_winusb/transfer.rs +++ b/src/platform/windows_winusb/transfer.rs @@ -6,7 +6,7 @@ use std::{ sync::Arc, }; -use log::{debug, error}; +use log::{debug, error, warn}; use windows_sys::Win32::{ Devices::Usb::{ WinUsb_ControlTransfer, WinUsb_GetOverlappedResult, WinUsb_ReadPipe, WinUsb_WritePipe, @@ -22,7 +22,7 @@ use windows_sys::Win32::{ use crate::transfer::{ notify_completion, Completion, ControlIn, ControlOut, EndpointType, PlatformSubmit, - PlatformTransfer, RequestBuffer, ResponseBuffer, TransferError, + PlatformTransfer, Recipient, RequestBuffer, ResponseBuffer, TransferError, }; use super::util::raw_handle; @@ -228,6 +228,11 @@ impl PlatformSubmit for TransferData { unsafe fn submit(&mut self, data: ControlIn, user_data: *mut c_void) { assert_eq!(self.endpoint, 0); assert_eq!(self.ep_type, EndpointType::Control); + + if data.recipient == Recipient::Interface && data.index as u8 != self.interface.interface { + warn!("WinUSB sends interface number instead of passed `index` when performing a control transfer with `Recipient::Interface`"); + } + addr_of_mut!((*self.event).ptr).write(user_data); let mut buf = ManuallyDrop::new(Vec::with_capacity(data.length as usize)); @@ -270,6 +275,11 @@ impl PlatformSubmit> for TransferData { unsafe fn submit(&mut self, data: ControlOut, user_data: *mut c_void) { assert_eq!(self.endpoint, 0); assert_eq!(self.ep_type, EndpointType::Control); + + if data.recipient == Recipient::Interface && data.index as u8 != self.interface.interface { + warn!("WinUSB sends interface number instead of passed `index` when performing a control transfer with `Recipient::Interface`"); + } + addr_of_mut!((*self.event).ptr).write(user_data); let mut buf = ManuallyDrop::new(data.data.to_vec());