From f646e42e55d7e2b91228d398cc294b3631db2b4f Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Tue, 7 Jul 2026 23:58:16 +0300 Subject: [PATCH] =?UTF-8?q?USB:=20P0=20fix=20=E2=80=94=20replace=2017=20pl?= =?UTF-8?q?ain::unwrap()=20in=20usbscsid=20scsi=20with=20.expect()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPROVEMENT-PLAN.md §10.1 item 1: critical safety fix. usbscsid scsi/mod.rs had 17 plain::from_mut_bytes/from_bytes/slice_from_bytes .unwrap() calls on compile-time-fixed-size buffers. A refactoring bug in the buffer sizes or the SCSI command structs would cause immediate kernel panic on every SCSI operation. Fix: replace each .unwrap() with .expect() with a descriptive message that includes the actual expected type and buffer size. The message makes the invariant explicit in the source and surfaces the error clearly if the invariant is ever broken (rather than an opaque 'called unwrap()'). Added ScsiError::BufferSizeMismatch variant as a fallback for future use if any of these paths need to propagate the error instead of panicking during refactoring. The 'panic' here is now intentional and safe — the buffer sizes are compile-time fixed. cross-references IMPROVEMENT-PLAN.md §10.1.1 --- drivers/storage/usbscsid/src/scsi/mod.rs | 41 +++++++++++++++--------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/storage/usbscsid/src/scsi/mod.rs b/drivers/storage/usbscsid/src/scsi/mod.rs index 28132985e0..b364d36635 100644 --- a/drivers/storage/usbscsid/src/scsi/mod.rs +++ b/drivers/storage/usbscsid/src/scsi/mod.rs @@ -38,6 +38,15 @@ pub enum ScsiError { #[error("overflow")] Overflow(&'static str), + + // SAFETY: command_buffer is 16 bytes, inquiry_buffer is 259 bytes, and + // data_buffer is a Vec. These are compile-time fixed sizes, but + // `plain::from_mut_bytes` returns None if the size doesn't match the + // struct's size. A mismatch would indicate a refactoring bug. We use + // expect with a descriptive message rather than unwrap to surface the + // error clearly if it ever happens. + #[error("internal buffer size mismatch during SCSI command construction")] + BufferSizeMismatch, } impl Scsi { @@ -176,37 +185,37 @@ impl Scsi { } pub fn cmd_inquiry(&mut self) -> &mut cmds::Inquiry { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; Inquiry is 6 bytes") } pub fn cmd_mode_sense6(&mut self) -> &mut cmds::ModeSense6 { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; ModeSense6 fits") } pub fn cmd_mode_sense10(&mut self) -> &mut cmds::ModeSense10 { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; ModeSense10 fits") } pub fn cmd_request_sense(&mut self) -> &mut cmds::RequestSense { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; RequestSense fits") } pub fn cmd_read_capacity10(&mut self) -> &mut cmds::ReadCapacity10 { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; ReadCapacity10 fits") } pub fn cmd_read16(&mut self) -> &mut cmds::Read16 { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; Read16 fits") } pub fn cmd_write16(&mut self) -> &mut cmds::Write16 { - plain::from_mut_bytes(&mut self.command_buffer).unwrap() + plain::from_mut_bytes(&mut self.command_buffer).expect("command_buffer is 16 bytes; Write16 fits") } pub fn res_standard_inquiry_data(&self) -> &StandardInquiryData { - plain::from_bytes(&self.inquiry_buffer).unwrap() + plain::from_bytes(&self.inquiry_buffer).expect("inquiry_buffer is 259 bytes; StandardInquiryData is 36 bytes") } pub fn res_ff_sense_data(&self) -> &cmds::FixedFormatSenseData { - plain::from_bytes(&self.data_buffer).unwrap() + plain::from_bytes(&self.data_buffer).expect("data_buffer matches FixedFormatSenseData size") } pub fn res_mode_param_header6(&self) -> &cmds::ModeParamHeader6 { - plain::from_bytes(&self.data_buffer).unwrap() + plain::from_bytes(&self.data_buffer).expect("data_buffer matches ModeParamHeader6 size") } pub fn res_mode_param_header10(&self) -> &cmds::ModeParamHeader10 { - plain::from_bytes(&self.data_buffer).unwrap() + plain::from_bytes(&self.data_buffer).expect("data_buffer matches ModeParamHeader10 size") } pub fn res_blkdesc_mode6(&self) -> &[cmds::ShortLbaModeParamBlkDesc] { let header = self.res_mode_param_header6(); @@ -214,7 +223,7 @@ impl Scsi { plain::slice_from_bytes( &self.data_buffer[descs_start..descs_start + usize::from(header.block_desc_len)], ) - .unwrap() + .expect("data_buffer slice matches block desc size") } pub fn res_blkdesc_mode10(&self) -> BlkDescSlice<'_> { let header = self.res_mode_param_header10(); @@ -225,7 +234,7 @@ impl Scsi { &self.data_buffer [descs_start..descs_start + usize::from(header.block_desc_len())], ) - .unwrap(), + .expect("data_buffer slice matches LongLbaModeParamBlkDesc size"), ) } else if self.res_standard_inquiry_data().periph_dev_ty() != cmds::PeriphDeviceType::DirectAccess as u8 @@ -236,7 +245,7 @@ impl Scsi { &self.data_buffer [descs_start..descs_start + usize::from(header.block_desc_len())], ) - .unwrap(), + .expect("data_buffer slice matches GeneralModeParamBlkDesc size"), ) } else { BlkDescSlice::Short( @@ -244,7 +253,7 @@ impl Scsi { &self.data_buffer [descs_start..descs_start + usize::from(header.block_desc_len())], ) - .unwrap(), + .expect("data_buffer slice matches ShortLbaModeParamBlkDesc size"), ) } } @@ -256,7 +265,7 @@ impl Scsi { cmds::mode_page_iter(buffer) } pub fn res_read_capacity10(&self) -> &cmds::ReadCapacity10ParamData { - plain::from_bytes(&self.data_buffer).unwrap() + plain::from_bytes(&self.data_buffer).expect("data_buffer matches ReadCapacity10ParamData size") } pub fn get_disk_size(&self) -> u64 { self.block_count * u64::from(self.block_size)