From 015fc393ba90136aab3c53d23208a1a71b7548b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dav=C3=AD=C3=B0=20Steinn=20Geirsson?= Date: Tue, 31 Mar 2026 15:31:27 +0000 Subject: [PATCH] fix: forward CLEAR_FEATURE(ENDPOINT_HALT) to real device for host passthrough When a real USB device stalls an endpoint, the VM sends CLEAR_FEATURE(ENDPOINT_HALT) to recover. Previously this was a no-op, leaving the endpoint permanently stalled and causing the guest to hang. Now the CLEAR_FEATURE handler in device.rs finds the interface that owns the stalled endpoint and calls clear_halt on its handler. For host passthrough this forwards to nusb's new Interface::clear_halt; for simulated devices it remains a no-op. Fixes YubiKey (and other CCID devices) hanging after endpoint stall during passthrough. Co-Authored-By: Claude Opus 4.6 (1M context) --- Cargo.lock | 10 ++--- cli/Cargo.toml | 2 +- flake.nix | 2 +- lib/Cargo.toml | 2 +- lib/src/device.rs | 90 +++++++++++++++++++++++++++++++++++++++++++- lib/src/host.rs | 5 +++ lib/src/interface.rs | 8 ++++ 7 files changed, 109 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 881e043..3d1432a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -393,9 +393,9 @@ dependencies = [ [[package]] name = "mio" -version = "1.1.1" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a69bcab0ad47271a0234d9422b131806bf3968021e5dc9328caf2d4cd58557fc" +checksum = "50b7e5b27aa02a74bac8c3f23f448f8d87ff11f92d3aac1a6ed369ee08cc56c1" dependencies = [ "libc", "wasi", @@ -438,7 +438,7 @@ dependencies = [ [[package]] name = "nusb" version = "0.2.3" -source = "git+https://git.dsg.is/dsg/nusb.git?rev=1239c676#1239c6765ab478b19b143544a467fadbb472197b" +source = "git+https://git.dsg.is/dsg/nusb.git?rev=a45a3e0#a45a3e08be93ce99ae90bb2551d0a7c8218c285c" dependencies = [ "core-foundation", "core-foundation-sys", @@ -759,9 +759,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "vsock" -version = "0.5.3" +version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b82aeb12ad864eb8cd26a6c21175d0bdc66d398584ee6c93c76964c3bcfc78ff" +checksum = "6ba782755fc073877e567c2253c0be48e4aa9a254c232d36d3985dfae0bd5205" dependencies = [ "libc", "nix", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 9b45984..30bdede 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -16,4 +16,4 @@ tokio-vsock = "0.7" tokio = { version = "1", features = ["rt-multi-thread", "macros", "signal", "time", "net"] } log = "0.4" env_logger = "0.11" -nusb = { git = "https://git.dsg.is/dsg/nusb.git", rev = "1239c676" } +nusb = { git = "https://git.dsg.is/dsg/nusb.git", rev = "a45a3e0" } diff --git a/flake.nix b/flake.nix index b7faa75..adb7eb1 100644 --- a/flake.nix +++ b/flake.nix @@ -35,7 +35,7 @@ src = self; - cargoHash = "sha256-ynwLW2FfZxr16KHaDgVUk8DlrXu5dKwS4pk1Rdo2jso="; + cargoHash = "sha256-gsxlVaaWhuAQ2cRMap3QRESutsK656x1RQPpyBnBQa0="; inherit nativeBuildInputs buildInputs; diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 6d58a0c..3213fe9 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -13,7 +13,7 @@ log = "0.4.17" num-traits = "0.2.15" num-derive = "0.4.2" serde = { version = "1.0", features = ["derive"], optional = true } -nusb = { git = "https://git.dsg.is/dsg/nusb.git", rev = "1239c676" } +nusb = { git = "https://git.dsg.is/dsg/nusb.git", rev = "a45a3e0" } tokio-util = { version = "0.7", features = ["rt"] } arbitrary = { version = "1", features = ["derive"], optional = true } diff --git a/lib/src/device.rs b/lib/src/device.rs index 7b14dd8..d16361c 100644 --- a/lib/src/device.rs +++ b/lib/src/device.rs @@ -722,8 +722,20 @@ impl UsbDevice { } } (0b00000010, Some(ClearFeature)) => { - // Endpoint recipient: no-op (simulated device doesn't stall) - debug!("Clear feature (endpoint)"); + let ep_addr = setup_packet.index as u8; + if ep_addr & 0x7F == 0 { + debug!("Clear feature on EP0 (no-op)"); + return Ok(UrbResponse::default()); + } + for state in &self.interface_states { + let inner = state.inner.read().await; + if inner.active.endpoints.iter().any(|e| e.address == ep_addr) { + debug!("Clear halt on endpoint {ep_addr:02x}"); + inner.active.handler.clear_halt(ep_addr)?; + return Ok(UrbResponse::default()); + } + } + warn!("Clear feature on unknown endpoint {ep_addr:02x}"); Ok(UrbResponse::default()) } (0b00000010, Some(SetFeature)) => { @@ -884,6 +896,80 @@ mod test { assert!(res.is_err()); } + #[test] + fn test_clear_feature_endpoint_halt() { + use std::sync::atomic::{AtomicU8, Ordering}; + setup_test_logger(); + + let cleared_ep = Arc::new(AtomicU8::new(0)); + + #[derive(Debug)] + struct ClearHaltTracker(Arc); + + impl UsbInterfaceHandler for ClearHaltTracker { + fn get_class_specific_descriptor(&self) -> Vec { + vec![] + } + fn handle_urb( + &self, + _interface: &UsbInterface, + _request: UrbRequest, + ) -> Result { + Ok(UrbResponse::default()) + } + fn clear_halt(&self, endpoint: u8) -> Result<()> { + self.0.store(endpoint, Ordering::SeqCst); + Ok(()) + } + fn as_any(&self) -> &dyn Any { + self + } + } + + let mut device = UsbDevice::new(0).unwrap(); + let handler = Arc::new(ClearHaltTracker(cleared_ep.clone())); + let ep_addr = 0x81u8; + + let interface = UsbInterface { + interface_class: 0xFF, + interface_subclass: 0, + interface_protocol: 0, + endpoints: vec![UsbEndpoint { + address: ep_addr, + attributes: 0x02, + max_packet_size: 512, + interval: 0, + class_specific_descriptor: vec![], + }], + string_interface: 0, + class_specific_descriptor: vec![], + handler: handler.clone(), + }; + device.interface_states.push(InterfaceState::new(interface)); + + let rt = tokio::runtime::Runtime::new().unwrap(); + let resp = rt + .block_on(device.handle_urb( + None, + UrbRequest { + ep: device.ep0_out.clone(), + transfer_buffer_length: 0, + setup: SetupPacket { + request_type: 0b00000010, + request: 1, + value: 0, + index: ep_addr as u16, + length: 0, + }, + ..Default::default() + }, + )) + .unwrap(); + + assert_eq!(resp.status, 0); + assert_eq!(cleared_ep.load(Ordering::SeqCst), ep_addr); + } + #[test] fn test_from_bytes_round_trip() { setup_test_logger(); diff --git a/lib/src/host.rs b/lib/src/host.rs index 9c21d77..4cf4726 100644 --- a/lib/src/host.rs +++ b/lib/src/host.rs @@ -43,6 +43,11 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { handle.set_alt_setting(alt).wait().map_err(Into::into) } + fn clear_halt(&self, endpoint: u8) -> Result<()> { + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); + handle.clear_halt(endpoint).wait().map_err(Into::into) + } + fn handle_urb( &self, _interface: &UsbInterface, diff --git a/lib/src/interface.rs b/lib/src/interface.rs index a72dbab..7bb077a 100644 --- a/lib/src/interface.rs +++ b/lib/src/interface.rs @@ -126,6 +126,14 @@ pub trait UsbInterfaceHandler: std::fmt::Debug + Send + Sync { Ok(()) } + /// Clear a halt condition on the given endpoint. + /// + /// For host passthrough, this forwards CLEAR_FEATURE(ENDPOINT_HALT) to + /// the real device. The default is a no-op for simulated devices. + fn clear_halt(&self, _endpoint: u8) -> Result<()> { + Ok(()) + } + /// Helper to downcast to actual struct fn as_any(&self) -> &dyn Any; }