From b7d6dd154534b358594d7e285d00d2a58ef21e25 Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Tue, 7 Jul 2026 07:44:59 +0300 Subject: [PATCH] =?UTF-8?q?usbscsid:=20P1-B=20=E2=80=94=20remove=20all=20p?= =?UTF-8?q?anic=20sites,=20return=20proper=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- drivers/storage/usbscsid/src/protocol/bot.rs | 36 +++++++++++++------- drivers/storage/usbscsid/src/scsi/mod.rs | 8 ++++- 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/drivers/storage/usbscsid/src/protocol/bot.rs b/drivers/storage/usbscsid/src/protocol/bot.rs index b751d51ad6..e3e08150ca 100644 --- a/drivers/storage/usbscsid/src/protocol/bot.rs +++ b/drivers/storage/usbscsid/src/protocol/bot.rs @@ -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( diff --git a/drivers/storage/usbscsid/src/scsi/mod.rs b/drivers/storage/usbscsid/src/scsi/mod.rs index 790abea68c..f7400a12d3 100644 --- a/drivers/storage/usbscsid/src/scsi/mod.rs +++ b/drivers/storage/usbscsid/src/scsi/mod.rs @@ -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