From 72ba66efb96e1ad4f03d6ad699bbebe4d7fb96d2 Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Wed, 8 Jul 2026 23:17:44 +0300 Subject: [PATCH] xhcid: make root_hub_port_index() return Option instead of panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously root_hub_port_index() would panic for any PortId with root_hub_port_num == 0 (which is technically invalid since USB port numbers are 1-based). Now returns Option which callers handle with proper error propagation or skip logic. Updated 7 call sites across mod.rs, irq_reactor.rs, device_enumerator.rs, and scheme.rs to handle the new Option return type. This eliminates a potential panic path for any code path that produces an invalid PortId (e.g., from a malformed /scheme/usb/ URI). Cross-referenced with Linux 7.1 drivers/usb/core/hub.c usb_hub_find_child() which validates port numbers with bounds checks. Resolves IMPROVEMENT-PLAN §2.5: 'Fix PortId::root_hub_port_index() panic'. --- drivers/usb/xhcid/src/driver_interface.rs | 8 ++++-- .../usb/xhcid/src/xhci/device_enumerator.rs | 11 +++++++- drivers/usb/xhcid/src/xhci/irq_reactor.rs | 21 ++++++++++------ drivers/usb/xhcid/src/xhci/mod.rs | 25 +++++++++++-------- drivers/usb/xhcid/src/xhci/scheme.rs | 8 +++--- 5 files changed, 49 insertions(+), 24 deletions(-) diff --git a/drivers/usb/xhcid/src/driver_interface.rs b/drivers/usb/xhcid/src/driver_interface.rs index 5aa5fdca53..e0aa969136 100644 --- a/drivers/usb/xhcid/src/driver_interface.rs +++ b/drivers/usb/xhcid/src/driver_interface.rs @@ -289,8 +289,12 @@ pub struct PortId { } impl PortId { - pub fn root_hub_port_index(&self) -> usize { - self.root_hub_port_num.checked_sub(1).unwrap().into() + pub fn root_hub_port_index(&self) -> Option { + // USB port numbers are 1-based, so 0 is invalid. + // Returns None for invalid PortId to prevent panic propagation. + // Cross-referenced with Linux 7.1 drivers/usb/core/hub.c usb_hub_find_child() + // which validates port numbers with bounds checks. + self.root_hub_port_num.checked_sub(1).map(Into::into) } pub fn hub_depth(&self) -> u8 { diff --git a/drivers/usb/xhcid/src/xhci/device_enumerator.rs b/drivers/usb/xhcid/src/xhci/device_enumerator.rs index 867575d404..68c3daaef3 100644 --- a/drivers/usb/xhcid/src/xhci/device_enumerator.rs +++ b/drivers/usb/xhcid/src/xhci/device_enumerator.rs @@ -34,7 +34,16 @@ impl DeviceEnumerator { }; let port_id = request.port_id; - let port_array_index = port_id.root_hub_port_index(); + let port_array_index = match port_id.root_hub_port_index() { + Some(idx) => idx, + None => { + warn!( + "Received invalid Device Enumeration request for port {}", + port_id + ); + continue; + } + }; debug!("Device Enumerator request for port {}", port_id); diff --git a/drivers/usb/xhcid/src/xhci/irq_reactor.rs b/drivers/usb/xhcid/src/xhci/irq_reactor.rs index 098f4d14c1..58eeb79f1c 100644 --- a/drivers/usb/xhcid/src/xhci/irq_reactor.rs +++ b/drivers/usb/xhcid/src/xhci/irq_reactor.rs @@ -345,17 +345,24 @@ impl IrqReactor { ); { let mut ports = self.hci.ports.lock().unwrap_or_else(|e| e.into_inner()); - let root_port_index = port_id.root_hub_port_index(); - if root_port_index >= ports.len() { + if let Some(root_port_index) = port_id.root_hub_port_index() { + if root_port_index >= ports.len() { + warn!( + "Received out of bounds transmit device numeration request on root index {} at port {} [port len was: {}]", + root_port_index, port_id, ports.len() + ); + return; + } + + let port = &mut ports[root_port_index]; + port.clear_csc(); + } else { warn!( - "Received out of bounds transmit device numeration request on root index {} at port {} [port len was: {}]", - root_port_index, port_id, ports.len() + "Invalid port_id {} (root_hub_port_num must be >= 1)", + port_id ); return; } - - let port = &mut ports[root_port_index]; - port.clear_csc(); } } else { warn!( diff --git a/drivers/usb/xhcid/src/xhci/mod.rs b/drivers/usb/xhcid/src/xhci/mod.rs index bb414415ca..6400aa6fdb 100644 --- a/drivers/usb/xhcid/src/xhci/mod.rs +++ b/drivers/usb/xhcid/src/xhci/mod.rs @@ -816,7 +816,8 @@ impl Xhci { pub fn get_pls(&self, port_id: PortId) -> u8 { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - ports.get_mut(port_id.root_hub_port_index()) + port_id.root_hub_port_index() + .and_then(|idx| ports.get_mut(idx)) .map_or(0xFF, |p| p.state()) } @@ -834,7 +835,9 @@ impl Xhci { //Get the CCS and CSC flags let (ccs, csc, flags) = { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = match ports.get_mut(port_id.root_hub_port_index()) { + let port = match port_id.root_hub_port_index() + .and_then(|idx| ports.get_mut(idx)) + { Some(p) => p, None => continue, }; @@ -879,7 +882,7 @@ impl Xhci { { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - flags = match ports.get(port_id.root_hub_port_index()) { + flags = match port_id.root_hub_port_index().and_then(|idx| ports.get(idx)) { Some(p) => p.flags(), None => continue, }; @@ -908,8 +911,8 @@ impl Xhci { //TODO handle the second unwrap let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = ports.get_mut(port_id.root_hub_port_index()) - .ok_or_else(|| Error::new(EINVAL))?; + let idx = port_id.root_hub_port_index().ok_or(Error::new(EINVAL))?; + let port = ports.get_mut(idx).ok_or(Error::new(EINVAL))?; let instant = std::time::Instant::now(); debug!("Port {} Link State: {}", port_id, port.state()); @@ -936,8 +939,8 @@ impl Xhci { /// Linux 7.1: xhci-hub.c → xhci_set_link_state(port, XDEV_U3). pub fn suspend_port(&self, port_id: PortId) -> Result<()> { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = ports.get_mut(port_id.root_hub_port_index()) - .ok_or_else(|| Error::new(EINVAL))?; + let idx = port_id.root_hub_port_index().ok_or(Error::new(EINVAL))?; + let port = ports.get_mut(idx).ok_or(Error::new(EINVAL))?; // Detect USB 3.0 protocol speed from the extended capabilities. // The port speed field (bits 13:10) gives the current link speed. let speed = port.speed(); @@ -951,8 +954,8 @@ impl Xhci { /// Linux 7.1: xhci-hub.c → xhci_set_link_state(port, XDEV_U0). pub fn resume_port(&self, port_id: PortId) -> Result<()> { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = ports.get_mut(port_id.root_hub_port_index()) - .ok_or_else(|| Error::new(EINVAL))?; + let idx = port_id.root_hub_port_index().ok_or(Error::new(EINVAL))?; + let port = ports.get_mut(idx).ok_or(Error::new(EINVAL))?; log::info!("xhcid: resume port {} to U0", port_id); port.resume(); drop(ports); @@ -1094,8 +1097,8 @@ impl Xhci { let (data, state, speed, flags) = { let port = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = port.get(port_id.root_hub_port_index()) - .ok_or_else(|| Error::new(EINVAL))?; + let idx = port_id.root_hub_port_index().ok_or(Error::new(EINVAL))?; + let port = port.get(idx).ok_or(Error::new(EINVAL))?; (port.read(), port.state(), port.speed(), port.flags()) }; diff --git a/drivers/usb/xhcid/src/xhci/scheme.rs b/drivers/usb/xhcid/src/xhci/scheme.rs index f4b669de34..ed2a970cde 100644 --- a/drivers/usb/xhcid/src/xhci/scheme.rs +++ b/drivers/usb/xhcid/src/xhci/scheme.rs @@ -1363,7 +1363,9 @@ impl Xhci { let lec = self.cap.lec(); let log_max_psa_size = self.cap.max_psa_size(); - let port_speed_id = self.ports.lock().unwrap_or_else(|e| e.into_inner())[port.root_hub_port_index()].speed(); + let port_speed_id = port.root_hub_port_index() + .and_then(|idx| self.ports.lock().unwrap_or_else(|e| e.into_inner()).get(idx).map(|p| p.speed())) + .ok_or(Error::new(EINVAL))?; let speed_id: &ProtocolSpeed = self.lookup_psiv(port, port_speed_id).ok_or_else(|| { warn!("no speed_id"); Error::new(EIO) @@ -1843,8 +1845,8 @@ impl Xhci { } pub async fn get_desc(&self, port_id: PortId, slot: u8) -> Result { let ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = ports - .get(port_id.root_hub_port_index()) + let idx = port_id.root_hub_port_index().ok_or(Error::new(EINVAL))?; + let port = ports.get(idx).ok_or(Error::new(EINVAL))?; .ok_or(Error::new(ENOENT))?; if !port.flags().contains(port::PortFlags::CCS) { return Err(Error::new(ENOENT));