drive/dvdnav: review follow-ups (ceiling doc/log, pread pre-check, linux FFI helpers, has_unlocker, EJECT_CDB, set-op consts)

This commit is contained in:
Matthew Jackson
2026-09-29 19:07:07 -07:00
parent 9f279cd184
commit 8ac8b53eb6
5 changed files with 78 additions and 51 deletions
Generated
-12
View File
@@ -850,15 +850,3 @@ dependencies = [
"log",
"simd-adler32",
]
[[patch.unused]]
name = "freemkv-engine"
version = "1.7.7"
[[patch.unused]]
name = "freemkv-i18n"
version = "1.7.7"
[[patch.unused]]
name = "freemkv-keysources"
version = "1.7.7"
+3 -1
View File
@@ -7,6 +7,8 @@
pub(crate) const PREVENT_ALLOW: u8 = 0x1E;
/// START STOP UNIT.
pub(crate) const START_STOP_UNIT: u8 = 0x1B;
/// START STOP UNIT with LoEj=1, Start=0: eject the disc if permitted (MMC-6 Table 633).
pub(crate) const EJECT_CDB: [u8; 6] = [START_STOP_UNIT, 0, 0, 0, 0x02, 0];
/// REPORT KEY.
pub(crate) const REPORT_KEY: u8 = 0xA4;
@@ -78,7 +80,7 @@ mod tests {
const ALLOW: [u8; 6] = [0x1E, 0, 0, 0, 0x00, 0];
const PREVENT: [u8; 6] = [0x1E, 0, 0, 0, 0x01, 0];
const EJECT: [u8; 6] = [0x1B, 0, 0, 0, 0x02, 0];
const EJECT: [u8; 6] = EJECT_CDB;
const START: [u8; 6] = [0x1B, 0, 0, 0, 0x01, 0];
const READ: [u8; 10] = [0x28, 0, 0, 0, 0, 0, 0, 0, 1, 0];
+23 -27
View File
@@ -597,8 +597,9 @@ impl Drive {
}
/// Wait for the drive to become ready: TEST UNIT READY every 500 ms until it
/// answers GOOD. Fails with `DeviceNotReady` only after 60 s without progress
/// (§2.11): an answer not yet seen in this wait, or a rising progress indicator.
/// answers GOOD. Fails with `DeviceNotReady` after 60 s without progress
/// (§2.11): an answer not yet seen in this wait, or a rising progress indicator;
/// or after an absolute 10 min ceiling, however much progress it shows.
/// A Stop interrupts between polls; a dead bus fails after 5 s of failures.
pub fn wait_ready(&mut self) -> Result<()> {
self.wait_ready_with(WaitReadyTiming::PRODUCTION)
@@ -610,6 +611,7 @@ impl Drive {
let t0 = std::time::Instant::now();
tracing::info!(target: "freemkv::drive", phase = "wait_ready", "begin");
let mut hb = crate::progress::Heartbeat::new("wait_ready");
let ceiling_hit: bool;
// T6: the answers seen so far and the highest progress indicator; either
// growing re-arms the no-progress window.
let moved = Progress::new();
@@ -694,7 +696,9 @@ impl Drive {
}
}
}
if stall.poll(&moved) == crate::halt::Stall::Expired || t0.elapsed() >= timing.ceiling {
let stalled = stall.poll(&moved) == crate::halt::Stall::Expired;
if stalled || t0.elapsed() >= timing.ceiling {
ceiling_hit = !stalled;
break;
}
self.pause(timing.poll)?;
@@ -704,7 +708,8 @@ impl Drive {
phase = "wait_ready",
elapsed_ms = t0.elapsed().as_millis() as u64,
window_ms = timing.window.as_millis() as u64,
"device never became ready: no progress for the whole window"
ceiling_hit,
"device never became ready: no progress for the window, or the ceiling was hit"
);
Err(Error::DeviceNotReady {
path: self.device_path.clone(),
@@ -1107,10 +1112,16 @@ impl Drive {
/// OEM VID through it (VID via the OEM path is decoupled from the host cert +
/// HRL). This mirrors [`Self::has_profile`] — the honest signal is "an
/// unlocker claims this drive" — rather than the old const `false`.
pub fn is_unlocked(&self) -> bool {
pub fn has_unlocker(&self) -> bool {
crate::unlock_bridge::unlocker_name(&self.drive_id).is_some()
}
/// Deprecated alias of [`Self::has_unlocker`].
#[deprecated(note = "use has_unlocker")]
pub fn is_unlocked(&self) -> bool {
self.has_unlocker()
}
/// Read sectors from the disc. Single-shot — no inline retries, no
/// SCSI reset.
///
@@ -1283,15 +1294,9 @@ impl Drive {
{
let len = count as usize * 2048;
let offset = lba as i64 * 2048;
// Drop the page cache for this region so the read hits the device.
// SAFETY: fd is the block fd this Drive owns; no pointers are passed.
let _ = unsafe {
libc::posix_fadvise(fd, offset, len as i64, libc::POSIX_FADV_DONTNEED)
};
// SAFETY: buf.len() >= len (checked above), so pread writes in bounds.
let n = unsafe {
libc::pread(fd, buf.as_mut_ptr() as *mut libc::c_void, len, offset)
};
// A Stop that landed since exec's check issues no blocking read.
self.check_token()?;
let n = crate::scsi::linux::pread_uncached(fd, &mut buf[..len], offset);
if n == len as isize {
// A Stop during the blocking read discards the data, as `exec` does.
self.check_token()?;
@@ -1426,7 +1431,7 @@ impl Drive {
pub fn eject(&mut self) -> Result<()> {
self.unlock_tray();
// SS-6 MMC-6 Table 633: LoEj 1, Start 0 = "Eject the disc if permitted".
let eject_cdb = [SCSI_START_STOP_UNIT, 0, 0, 0, 0x02, 0];
let eject_cdb = allow::EJECT_CDB;
let mut buf = [0u8; 0];
self.exec(
&eject_cdb,
@@ -1443,7 +1448,7 @@ impl Drive {
pub(crate) fn finish_eject(&mut self) -> Result<()> {
self.unlock_tray();
// SS-6 MMC-6 Table 633: LoEj 1, Start 0 = "Eject the disc if permitted".
let eject_cdb = [SCSI_START_STOP_UNIT, 0, 0, 0, 0x02, 0];
let eject_cdb = allow::EJECT_CDB;
let mut buf = [0u8; 0];
let dir = crate::scsi::DataDirection::None;
self.exec_cleanup(&eject_cdb, dir, &mut buf, 30_000, CleanupCtx::FinishEject)?;
@@ -1505,8 +1510,7 @@ impl Drop for Drive {
// SgIoTransport::drop() runs next, calling libc::close(fd)
#[cfg(target_os = "linux")]
if let Some(fd) = self.block_dev_fd.take() {
// SAFETY: fd was opened by this Drive and is taken (closed once) here.
unsafe { libc::close(fd) };
crate::scsi::linux::close_block_fd(fd);
}
}
}
@@ -1526,15 +1530,7 @@ fn open_block_device_for_sg(sg_path: &Path) -> Option<std::os::unix::io::RawFd>
.find_map(|e| e.file_name().into_string().ok())?;
let block_path = format!("/dev/{}", block_name);
let mut bytes = block_path.as_bytes().to_vec();
bytes.push(0);
// SAFETY: bytes is NUL-terminated and outlives the call.
let fd = unsafe {
libc::open(
bytes.as_ptr() as *const libc::c_char,
libc::O_RDONLY | libc::O_CLOEXEC,
)
};
let fd = crate::scsi::linux::open_block_ro(&block_path);
if fd < 0 {
tracing::debug!(
target: "freemkv::drive",
+24 -11
View File
@@ -47,6 +47,19 @@ const CMP_GT: u8 = 5;
const CMP_LE: u8 = 6;
const CMP_LT: u8 = 7;
// Set-op codes (libdvdnav eval_set_op).
const SET_MOV: u8 = 1;
const SET_SWAP: u8 = 2;
const SET_ADD: u8 = 3;
const SET_SUB: u8 = 4;
const SET_MUL: u8 = 5;
const SET_DIV: u8 = 6;
const SET_MOD: u8 = 7;
const SET_RND: u8 = 8;
const SET_AND: u8 = 9;
const SET_OR: u8 = 10;
const SET_XOR: u8 = 11;
/// Maximum TT_SRPT entries honoured — the DVD-Video 99-title format maximum
/// (the on-disc count is an untrusted u16). Shared with the IFO parser so the
/// two can never diverge.
@@ -181,23 +194,23 @@ impl Vm {
// Each arm sets (new value, new taint). `mov` overwrites with the source's
// taint alone (clearing prior taint); accumulating ops union both.
let (nv, nt) = match op {
1 => (v, src_tainted), // mov
2 => {
SET_MOV => (v, src_tainted), // mov
SET_SWAP => {
// swap: reg2 (byte5 low nibble) takes the old value first.
let reg2 = (src & 0x0F) as usize;
self.gprm[reg2] = cur;
self.gprm_tainted[reg2] = cur_tainted;
(v, src_tainted)
}
3 => (cur.saturating_add(v), t),
4 => (cur.saturating_sub(v), t),
5 => (cur.saturating_mul(v), t), // libdvdnav's i32 product overflows (C UB) past 0x7FFF_FFFF
6 => (cur.checked_div(v).unwrap_or(0xFFFF), t),
7 => (cur.checked_rem(v).unwrap_or(0xFFFF), t),
8 => (cur, true), // rnd: non-deterministic
9 => (cur & v, t),
10 => (cur | v, t),
11 => (cur ^ v, t),
SET_ADD => (cur.saturating_add(v), t),
SET_SUB => (cur.saturating_sub(v), t),
SET_MUL => (cur.saturating_mul(v), t), // libdvdnav's i32 product overflows (C UB) past 0x7FFF_FFFF
SET_DIV => (cur.checked_div(v).unwrap_or(0xFFFF), t),
SET_MOD => (cur.checked_rem(v).unwrap_or(0xFFFF), t),
SET_RND => (cur, true), // rnd: non-deterministic
SET_AND => (cur & v, t),
SET_OR => (cur | v, t),
SET_XOR => (cur ^ v, t),
_ => (cur, self.gprm_tainted[idx]), // unknown op: no-op
};
self.gprm[idx] = nv;
+28
View File
@@ -663,6 +663,34 @@ pub(super) fn disc_presence(path: &Path) -> Result<super::DiscPresence> {
// After a transport failure the fd is reopened in the background and the next
// execute() adopts it. Needs a real device: FREEMKV_TEST_SG_DEVICE (default sg2).
/// Open a block device read-only (no O_DIRECT). Negative on failure.
pub(crate) fn open_block_ro(path: &str) -> i32 {
let mut bytes = path.as_bytes().to_vec();
bytes.push(0);
// SAFETY: bytes is NUL-terminated and outlives the call.
unsafe {
libc::open(
bytes.as_ptr() as *const libc::c_char,
libc::O_RDONLY | libc::O_CLOEXEC,
)
}
}
/// Close a block fd opened by [`open_block_ro`]; the caller must not reuse it.
pub(crate) fn close_block_fd(fd: i32) {
// SAFETY: the caller owns fd and passes it here exactly once.
unsafe { libc::close(fd) };
}
/// Drop the page cache for the range, then `pread` into `buf` at `offset`.
/// Returns the byte count, or a negative value on error (errno set).
pub(crate) fn pread_uncached(fd: i32, buf: &mut [u8], offset: i64) -> isize {
// SAFETY: fd is a live block fd; fadvise passes no pointers.
let _ = unsafe { libc::posix_fadvise(fd, offset, buf.len() as i64, libc::POSIX_FADV_DONTNEED) };
// SAFETY: buf is a valid writable slice of buf.len() bytes.
unsafe { libc::pread(fd, buf.as_mut_ptr() as *mut libc::c_void, buf.len(), offset) }
}
#[cfg(test)]
mod recovery_device_tests {
use super::*;