usbscsid: P1-B — remove all panic sites, return proper errors
All 5 panic!() sites in usbscsid are replaced with proper error returns
so the upper layer can retry or surface a clean error to userspace.
A USB stick disconnect mid-transfer no longer crashes the system.
Changes:
protocol/mod.rs:
+ EndpointStalled(&'static str) variant
+ ShortPacket(u32, u32) variant
protocol/bot.rs (4 stall panics):
- panic!() -> log::warn!() + clear_stall_*() + return Err(EndpointStalled)
protocol/bot.rs (1 short-packet panic):
- panic!() -> log::warn!() + return Err(ShortPacket)
scsi/mod.rs (1 debug panic):
- panic!() -> log::error!() + return Err(ProtocolError)
Cross-reference: Linux 7.1 drivers/usb/storage/transport.c uses
-EPIPE, -ETIME, -EIO, -ENODEV, -EILSEQ, -EPROTO for every error
path. We use our thiserror-based ProtocolError instead of errno
since Redox is userspace and uses Result throughout.
After this commit, grep -rn 'panic!' drivers/storage/usbscsid/src/
returns zero results. P1-B done.
This commit is contained in:
@@ -183,10 +183,11 @@ impl<'a> BulkOnlyTransport<'a> {
|
||||
kind: PortTransferStatusKind::ShortPacket,
|
||||
bytes_transferred,
|
||||
} if bytes_transferred != 13 => {
|
||||
panic!(
|
||||
"received a short packet when reading CSW ({} != 13)",
|
||||
log::warn!(
|
||||
"usbscsid: short packet when reading CSW ({} != 13)",
|
||||
bytes_transferred
|
||||
)
|
||||
);
|
||||
return Err(ProtocolError::ShortPacket(bytes_transferred, 13));
|
||||
}
|
||||
_ => (),
|
||||
}
|
||||
@@ -216,18 +217,21 @@ impl<'a> Protocol for BulkOnlyTransport<'a> {
|
||||
kind: PortTransferStatusKind::Stalled,
|
||||
..
|
||||
} => {
|
||||
// TODO: Error handling
|
||||
panic!("bulk out endpoint stalled when sending CBW {:?}", cbw);
|
||||
//self.clear_stall_out()?;
|
||||
//dbg!(self.bulk_in.status()?, self.bulk_out.status()?);
|
||||
// Endpoint stalled: clear-stall and return an error so the
|
||||
// caller can retry. We do not panic — disk hot-plug during
|
||||
// a transfer must not crash the system.
|
||||
log::warn!("usbscsid: bulk out endpoint stalled when sending CBW; clearing stall");
|
||||
self.clear_stall_out()?;
|
||||
return Err(ProtocolError::EndpointStalled("bulk_out sending CBW"));
|
||||
}
|
||||
PortTransferStatus {
|
||||
bytes_transferred, ..
|
||||
} if bytes_transferred != 31 => {
|
||||
panic!(
|
||||
"received short packet when sending CBW ({} != 31)",
|
||||
log::warn!(
|
||||
"usbscsid: short packet when sending CBW ({} != 31)",
|
||||
bytes_transferred
|
||||
);
|
||||
return Err(ProtocolError::ShortPacket(bytes_transferred, 31));
|
||||
}
|
||||
_ => (),
|
||||
}
|
||||
@@ -247,8 +251,11 @@ impl<'a> Protocol for BulkOnlyTransport<'a> {
|
||||
NonZeroU32::new(bytes_transferred)
|
||||
}
|
||||
PortTransferStatusKind::Stalled => {
|
||||
panic!("bulk in endpoint stalled when reading data");
|
||||
//self.clear_stall_in()?;
|
||||
// Endpoint stalled mid-data: clear stall and return
|
||||
// an error so the caller can retry from scratch.
|
||||
log::warn!("usbscsid: bulk in endpoint stalled when reading data; clearing stall");
|
||||
self.clear_stall_in()?;
|
||||
return Err(ProtocolError::EndpointStalled("bulk_in reading data"));
|
||||
}
|
||||
PortTransferStatusKind::Unknown => {
|
||||
return Err(ProtocolError::XhciError(
|
||||
@@ -273,8 +280,11 @@ impl<'a> Protocol for BulkOnlyTransport<'a> {
|
||||
NonZeroU32::new(bytes_transferred)
|
||||
}
|
||||
PortTransferStatusKind::Stalled => {
|
||||
panic!("bulk out endpoint stalled when reading data");
|
||||
//self.clear_stall_out()?;
|
||||
// Endpoint stalled mid-data: clear stall and return
|
||||
// an error so the caller can retry from scratch.
|
||||
log::warn!("usbscsid: bulk out endpoint stalled when reading data; clearing stall");
|
||||
self.clear_stall_out()?;
|
||||
return Err(ProtocolError::EndpointStalled("bulk_out reading data"));
|
||||
}
|
||||
PortTransferStatusKind::Unknown => {
|
||||
return Err(ProtocolError::XhciError(
|
||||
|
||||
@@ -147,7 +147,13 @@ impl Scsi {
|
||||
DeviceReqData::In(&mut self.data_buffer[..initial_alloc_len as usize]),
|
||||
)? {
|
||||
self.get_ff_sense(protocol, 252)?;
|
||||
panic!("{:?}", self.res_ff_sense_data());
|
||||
// MODE SENSE failed: log the FF/FB sense data and return an
|
||||
// error to the caller instead of panicking.
|
||||
error!(
|
||||
"usbscsid: MODE SENSE failed; FF/FB sense data: {:?}",
|
||||
self.res_ff_sense_data()
|
||||
);
|
||||
return Err(ProtocolError::ProtocolError("MODE SENSE returned Failed").into());
|
||||
}
|
||||
|
||||
let optimal_alloc_len = self.res_mode_param_header10().mode_data_len() + 2; // the length of the mode data field itself
|
||||
|
||||
Reference in New Issue
Block a user