unlock_bridge: map non-SCSI transport faults (IoError/DeviceNotFound) to 0xFF
Round-2 audit gap in the prior bridge fix: extract_scsi_context only recognizes Error::ScsiError / DiscRead and collapses every other variant to (0, None). But the Linux SG_IO transport returns genuine dead-bus faults as non-SCSI variants — Error::IoError (ioctl(SG_IO) == -1: ENODEV/EIO on an unplugged bridge) and Error::DeviceNotFound (fd gone) — which were crossing the seam as status 0x00, so the unlock crate classified neither as a transport fault (needs 0xFF) nor as a sense-bearing rejection, and kept hammering a dead bus. Match the error variant in the adapter: a SCSI status (CHECK CONDITION or a drive-tagged 0xFF) keeps its real status + sense; any other variant is a non-SCSI transport/IO fault → SCSI_STATUS_TRANSPORT_FAILURE / None. Keying off `sense.is_none()` would be wrong (a senseless CHECK CONDITION is a rejection). Add bridge tests covering CHECK-CONDITION sense preservation, 0xFF passthrough, and IoError/DeviceNotFound → 0xFF.
This commit is contained in:
+97
-1
@@ -56,7 +56,22 @@ impl fu::scsi::ScsiTransport for ScsiAdapter<'_> {
|
|||||||
// genuine transport fault (status 0xFF, no sense) maps through
|
// genuine transport fault (status 0xFF, no sense) maps through
|
||||||
// unchanged.
|
// unchanged.
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
let (status, sense) = crate::drive::extract_scsi_context(&e);
|
// A SCSI status (CHECK CONDITION or a 0xFF transport fault the
|
||||||
|
// drive layer already tagged) carries its real status + sense via
|
||||||
|
// extract_scsi_context. Any OTHER error variant is a non-SCSI
|
||||||
|
// transport/IO-layer fault — ioctl(SG_IO) == -1 (ENODEV/EIO on an
|
||||||
|
// unplugged bridge) or the fd is gone — i.e. a DEAD BUS, not a
|
||||||
|
// drive rejection; surface the transport-failure status so the
|
||||||
|
// unlock crate bails instead of hammering a wedged device.
|
||||||
|
// (Keying off `sense.is_none()` would be wrong: a CHECK CONDITION
|
||||||
|
// whose sense didn't parse is a rejection, not a transport fault.)
|
||||||
|
let (status, sense) = match &e {
|
||||||
|
crate::error::Error::ScsiError { .. }
|
||||||
|
| crate::error::Error::DiscRead { .. } => {
|
||||||
|
crate::drive::extract_scsi_context(&e)
|
||||||
|
}
|
||||||
|
_ => (crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE, None),
|
||||||
|
};
|
||||||
let sense_buf = sense.map(|s| {
|
let sense_buf = sense.map(|s| {
|
||||||
let mut b = [0u8; 32];
|
let mut b = [0u8; 32];
|
||||||
b[2] = s.sense_key & 0x0F;
|
b[2] = s.sense_key & 0x0F;
|
||||||
@@ -108,3 +123,84 @@ pub(crate) fn run_unlockers(
|
|||||||
}
|
}
|
||||||
Err(fu::UnlockError::NotApplicable)
|
Err(fu::UnlockError::NotApplicable)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use freemkv_unlock::scsi::ScsiTransport as _; // brings `execute` into scope
|
||||||
|
|
||||||
|
/// A fake libfreemkv transport whose `execute` always fails with a
|
||||||
|
/// freshly-built error (`crate::error::Error` isn't `Clone` — `io::Error`).
|
||||||
|
struct ErrTransport<F>(F);
|
||||||
|
impl<F: FnMut() -> crate::error::Error + Send> crate::scsi::ScsiTransport for ErrTransport<F> {
|
||||||
|
fn execute(
|
||||||
|
&mut self,
|
||||||
|
_cdb: &[u8],
|
||||||
|
_dir: crate::scsi::DataDirection,
|
||||||
|
_data: &mut [u8],
|
||||||
|
_timeout_ms: u32,
|
||||||
|
) -> std::result::Result<crate::scsi::ScsiResult, crate::error::Error> {
|
||||||
|
Err((self.0)())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn adapt(e: impl FnMut() -> crate::error::Error + Send) -> fu::scsi::ScsiError {
|
||||||
|
let mut t = ErrTransport(e);
|
||||||
|
let mut adapter = ScsiAdapter(&mut t);
|
||||||
|
let mut buf = [0u8; 0];
|
||||||
|
adapter
|
||||||
|
.execute(&[0u8; 12], fu::scsi::DataDirection::None, &mut buf, 1_000)
|
||||||
|
.expect_err("error path")
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A CHECK CONDITION carrying sense crosses the seam with status + parsed
|
||||||
|
/// sense intact, so the unlock crate's ILLEGAL_REQUEST wedge guard can fire.
|
||||||
|
#[test]
|
||||||
|
fn check_condition_preserves_status_and_sense() {
|
||||||
|
let err = adapt(|| crate::error::Error::ScsiError {
|
||||||
|
opcode: 0xA3,
|
||||||
|
status: 0x02,
|
||||||
|
sense: Some(crate::scsi::ScsiSense {
|
||||||
|
sense_key: 0x05,
|
||||||
|
asc: 0x24,
|
||||||
|
ascq: 0x00,
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
assert_eq!(err.status, 0x02);
|
||||||
|
let sense = err.sense.expect("sense preserved");
|
||||||
|
assert!(fu::scsi::ScsiSense::from_buf(&sense).is_illegal_request());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A drive-tagged transport fault (status 0xFF) crosses unchanged.
|
||||||
|
#[test]
|
||||||
|
fn scsi_transport_fault_maps_unchanged() {
|
||||||
|
let err = adapt(|| crate::error::Error::ScsiError {
|
||||||
|
opcode: 0,
|
||||||
|
status: crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE,
|
||||||
|
sense: None,
|
||||||
|
});
|
||||||
|
assert_eq!(err.status, 0xFF);
|
||||||
|
assert!(err.sense.is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A non-SCSI IO fault (ioctl SG_IO == -1: ENODEV/EIO) is a dead bus —
|
||||||
|
/// surfaced as 0xFF so the unlock crate bails instead of hammering it.
|
||||||
|
#[test]
|
||||||
|
fn io_error_maps_to_transport_failure() {
|
||||||
|
let err = adapt(|| crate::error::Error::IoError {
|
||||||
|
source: std::io::Error::from(std::io::ErrorKind::NotConnected),
|
||||||
|
});
|
||||||
|
assert_eq!(err.status, crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE);
|
||||||
|
assert!(err.sense.is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Device-gone (fd closed) likewise maps to the transport-failure status.
|
||||||
|
#[test]
|
||||||
|
fn device_not_found_maps_to_transport_failure() {
|
||||||
|
let err = adapt(|| crate::error::Error::DeviceNotFound {
|
||||||
|
path: "/dev/sg9".into(),
|
||||||
|
});
|
||||||
|
assert_eq!(err.status, 0xFF);
|
||||||
|
assert!(err.sense.is_none());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user