Make five tests capable of failing, and stop the presence probe unmounting the disc
The worst of the five was a regression suite that never touched the code it guarded: nine batch-count tests called `safe_batch_count` and `buggy_batch_count`, both defined in the test file itself. The u16 truncation they exist to prevent could be reintroduced in sector/prefetched.rs with every one of them green. They now drive the real producer through the public API, and reinstating the truncation fails five of the nine. Worth recording that the symptom has changed since the original fix: the unit-alignment clamp below floors a zero batch at three sectors, so the bug is now a twenty-fold throughput cliff rather than the stall it once was. The MP4 reserve test's only numeric case was dominated by the floor and the buffer, so BYTES_PER_SAMPLE could be zeroed without failing it. It now has a case where the per-sample term dominates. The zero-count guard in FileSectorSource was likewise unfalsifiable — seek-past-EOF and a zero-length read both succeed — so the test now observes the file cursor. The AACS media-key ambiguity guard had no test at all; the pool scan is extracted so the verifier can be injected, because a genuine two-key collision needs one ciphertext decrypting under two AES-128 keys to plaintexts sharing a 64-bit magic, which is a 2^64 search and not a fixture. macOS implemented the documented cheap, side-effect-free presence probe by building a full exclusive transport — which force-unmounts the disc. Linux and Windows issue one TEST UNIT READY with no unmount; macOS was the outlier. It now walks the IOKit registry for the media object instead. The C shim's registry reads assumed CoreFoundation types the registry does not guarantee, so a driver publishing a CFNumber where a CFString was expected aborted the process from inside public API. Types are checked and a wrong type treated as absent. The unbounded waitpid on the unmount child is now a polled deadline, and the last-resort match gained the NULL check its two siblings already had. The empty-CDB guard existed only on Linux while a shared helper's comment claimed all three backends had it. Moved into the helper, so the comment is now true and macOS and Windows are covered. One finding was REJECTED with evidence rather than fixed. The TrueHD buffer-cap test was indeed bogus, but MAX_TRUEHD_BUF turns out to be unreachable by any input: the parser only retains data when the buffer is shorter than the declared AU, and that declaration is twelve bits, so the worst case is 8189 bytes against a 256 KiB cap. An exhaustive sweep over all 65536 AU headers confirmed it. The fixture now sits at the reachable ceiling and asserts that instead. The cap itself is left in place as defence, unreachable by construction, matching how the AC-3 resync guard was handled earlier in this audit. Two behaviour changes worth naming: Linux's empty-CDB error becomes InvalidCdbLength rather than a transport failure, and an unknown device now reports absent media rather than a not-found error, because the registry cannot tell an empty drive from a missing one. The latter is a conflation of the kind this audit has fixed three times; it is recorded for the next round rather than left silent.
This commit is contained in:
+37
-4
@@ -41,6 +41,11 @@ pub const AACS_KEY_CLASS: u8 = 0x02;
|
||||
/// TUR is the cheapest SCSI op (no data transfer); 5 s is generous
|
||||
/// for any healthy bus and short enough that a hung device can't stall
|
||||
/// a poll-loop tick.
|
||||
///
|
||||
/// Used by the Linux and Windows backends. macOS answers the same question
|
||||
/// from the IOKit registry (no SCSI command is issued, so no timeout applies)
|
||||
/// — see `macos::drive_has_disc`.
|
||||
#[cfg_attr(target_os = "macos", allow(dead_code))]
|
||||
pub(crate) const TUR_TIMEOUT_MS: u32 = 5_000;
|
||||
|
||||
/// Timeout for content READ commands (READ_10 / READ_12) on the fast
|
||||
@@ -117,8 +122,14 @@ pub const SCSI_STATUS_TRANSPORT_FAILURE: u8 = 0xFF;
|
||||
/// Lives here, shared by all three platform backends, so the guard cannot
|
||||
/// drift per platform (it previously truncated on Linux and Windows while
|
||||
/// erroring on macOS — the "works on my platform, not theirs" class).
|
||||
/// An EMPTY CDB is rejected here too. `ScsiTransport` is a public trait, so an
|
||||
/// out-of-crate caller can pass one; every backend then either indexes `cdb[0]`
|
||||
/// (a panic out of a public API) or hands the driver a zero-length command
|
||||
/// descriptor, which under SPC-4 is not a command at all. That guard used to
|
||||
/// exist ONLY in the Linux backend — macOS and Windows had nothing — which is
|
||||
/// the same per-platform drift this helper exists to prevent.
|
||||
pub(crate) fn checked_cdb_len(cdb: &[u8], max: usize) -> Result<u8> {
|
||||
if cdb.len() > max {
|
||||
if cdb.is_empty() || cdb.len() > max {
|
||||
return Err(Error::InvalidCdbLength {
|
||||
len: cdb.len(),
|
||||
max,
|
||||
@@ -165,11 +176,10 @@ mod cdb_len_tests {
|
||||
}
|
||||
|
||||
/// Every real CDB length (SPC-4 groups 0-5: 6, 10, 12, 16 bytes) is
|
||||
/// accepted and reported verbatim, and an empty CDB reports 0 — the
|
||||
/// backends' own empty-CDB guards handle that case.
|
||||
/// accepted and reported verbatim.
|
||||
#[test]
|
||||
fn in_range_cdb_lengths_pass_through_verbatim() {
|
||||
for len in [0usize, 6, 10, 12, 16] {
|
||||
for len in [6usize, 10, 12, 16] {
|
||||
let cdb = vec![0u8; len];
|
||||
assert_eq!(
|
||||
checked_cdb_len(&cdb, MAX).ok(),
|
||||
@@ -178,6 +188,29 @@ mod cdb_len_tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// An EMPTY CDB must be rejected by the SHARED helper, not left to a
|
||||
/// per-backend guard. It previously reported `Ok(0)` and only the Linux
|
||||
/// backend caught it before `cdb[0]`; macOS and Windows passed a
|
||||
/// zero-length command descriptor straight to the driver.
|
||||
///
|
||||
/// This is the only place the property can be tested on every platform's
|
||||
/// CI — none of `linux.rs` / `macos.rs` / `windows.rs` compiles on more
|
||||
/// than one host.
|
||||
#[test]
|
||||
fn empty_cdb_is_rejected_by_the_shared_helper() {
|
||||
match checked_cdb_len(&[], MAX) {
|
||||
Err(Error::InvalidCdbLength { len, max }) => {
|
||||
assert_eq!(len, 0);
|
||||
assert_eq!(max, MAX);
|
||||
}
|
||||
Err(other) => panic!("expected InvalidCdbLength, got {other:?}"),
|
||||
Ok(n) => panic!(
|
||||
"empty CDB accepted with length {n} — every backend would then \
|
||||
index cdb[0] or issue a zero-length command descriptor"
|
||||
),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── SPC-4 sense keys (§4.5.6 Table 28) ─────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user