xhcid: make root_hub_port_index() return Option<usize> instead of panicking
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<usize> 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'.
This commit is contained in:
@@ -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<usize> {
|
||||
// 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 {
|
||||
|
||||
@@ -34,7 +34,16 @@ impl<const N: usize> DeviceEnumerator<N> {
|
||||
};
|
||||
|
||||
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);
|
||||
|
||||
|
||||
@@ -345,17 +345,24 @@ impl<const N: usize> IrqReactor<N> {
|
||||
);
|
||||
{
|
||||
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!(
|
||||
|
||||
@@ -816,7 +816,8 @@ impl<const N: usize> Xhci<N> {
|
||||
|
||||
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<const N: usize> Xhci<N> {
|
||||
//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<const N: usize> Xhci<N> {
|
||||
{
|
||||
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<const N: usize> Xhci<N> {
|
||||
|
||||
//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<const N: usize> Xhci<N> {
|
||||
/// 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<const N: usize> Xhci<N> {
|
||||
/// 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<const N: usize> Xhci<N> {
|
||||
|
||||
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())
|
||||
};
|
||||
|
||||
|
||||
@@ -1363,7 +1363,9 @@ impl<const N: usize> Xhci<N> {
|
||||
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<const N: usize> Xhci<N> {
|
||||
}
|
||||
pub async fn get_desc(&self, port_id: PortId, slot: u8) -> Result<DevDesc> {
|
||||
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));
|
||||
|
||||
Reference in New Issue
Block a user