fix(udf): file_start_lba must skip a leading unrecorded extent

Regression from the round-8 change that started RETAINING ECMA-167
4/14.14.1.1 type-1 (allocated, not recorded) descriptors. Retaining them
is correct — dropping one slides every later extent's data down by the
hole's length, corrupting the file silently. But read_icb_extent still
took extents.first(), so the value it returns can now be a hole.

A type-1 extent's lba is where SPACE is allocated, not where bytes live.
IcbExtent's own doc says exactly that. file_start_lba hands the value
out as "the absolute starting LBA of a file's first data extent", and
ifo.rs uses it as the base for every VTS VOB extent:

    file_start_lba(IFO) + vtstt_vobs + cell.first_sector

So a DVD whose IFO's first descriptor is type-1 reads its entire video
title set from the wrong place on disc. No error anywhere — the reads
succeed, they just land on unrelated sectors. Verified: the test reports
2900 instead of 2040 with the old code.

Same shape as file_extents/extents_abs_at dropping the recorded flag,
and the same root cause: one change taught read_icb_extents about a new
extent type and did not visit the accessors that consume its output.
Three of them; two are still open (task filed).

Found by the round-9 opus escalation over the API contract, dispatched
because the sonnet pass over the same scope returned zero findings.
Seven of its eight items were absences rather than wrong lines — the
class a wrong-line scan structurally cannot see.
This commit is contained in:
Matthew Jackson
2026-07-30 20:00:15 -07:00
parent 6868b93b7e
commit 54d038e478
+70 -1
View File
@@ -442,9 +442,27 @@ impl UdfFs {
/// Read an Extended File Entry (tag 266) or File Entry (tag 261) /// Read an Extended File Entry (tag 266) or File Entry (tag 261)
/// and return its first allocation extent: (data_lba, data_length). /// and return its first allocation extent: (data_lba, data_length).
/// The data_lba is partition-relative. /// The data_lba is partition-relative.
/// The file's first RECORDED extent — where its readable data actually
/// begins.
///
/// Not `extents.first()`. `read_icb_extents` retains ECMA-167 4/14.14.1.1
/// type-1 (allocated, not recorded) descriptors, because dropping one would
/// slide every later extent's data down by the hole's length. But a type-1
/// extent's `lba` is where SPACE is allocated, not where bytes live, and
/// this accessor's one caller — `file_start_lba` — hands its result out as
/// "the absolute starting LBA of a file's first data extent".
///
/// `ifo.rs` then uses that as the base for every VTS VOB extent
/// (`file_start_lba(IFO) + vtstt_vobs + cell.first_sector`), so a file whose
/// FIRST descriptor is type-1 would put the entire video title set at the
/// wrong place on disc, with no error anywhere.
fn read_icb_extent(&self, reader: &mut dyn SectorSource, meta_lba: u32) -> Result<IcbExtent> { fn read_icb_extent(&self, reader: &mut dyn SectorSource, meta_lba: u32) -> Result<IcbExtent> {
let extents = self.read_icb_extents(reader, meta_lba)?; let extents = self.read_icb_extents(reader, meta_lba)?;
extents.first().copied().ok_or(Error::DiscRead { extents
.iter()
.find(|e| e.recorded)
.copied()
.ok_or(Error::DiscRead {
// Diagnostic sector only; meta_to_abs can overflow on a crafted // Diagnostic sector only; meta_to_abs can overflow on a crafted
// meta_lba, in which case 0 is a harmless placeholder for the // meta_lba, in which case 0 is a harmless placeholder for the
// error-context field. // error-context field.
@@ -1891,6 +1909,57 @@ mod tests {
s s
} }
/// `file_start_lba` must skip a leading UNRECORDED extent and report where
/// the file's DATA starts.
///
/// ECMA-167 4/14.14.1.1 type 1 is allocated-but-not-recorded: the space
/// belongs to the file and occupies its byte range, but nothing was written
/// there and its `lba` is where SPACE lives, not bytes. `read_icb_extents`
/// retains such descriptors — dropping one would slide every later extent's
/// data down by the hole's length — so `.first()` can now be one.
///
/// `ifo.rs` uses this value as the base for every VTS VOB extent
/// (`file_start_lba(IFO) + vtstt_vobs + cell.first_sector`), so getting it
/// wrong puts the entire video title set at the wrong place on disc, with
/// no error anywhere — a silent wrong answer on an ordinary DVD.
#[test]
fn file_start_lba_skips_a_leading_unrecorded_extent() {
use fixture::{DirSpec, MemDisc, PART_START, build_udf_skeleton, lay_dir};
const HOLE_LBA: u32 = 900;
const DATA_LBA: u32 = 40;
let mut disc = MemDisc::new();
build_udf_skeleton(&mut disc, 10);
lay_dir(
&mut disc,
&DirSpec {
name: String::new(),
icb_lba: 10,
dir_data_lba: 11,
files: vec![fixture::file("VIDEO_TS.IFO", 12, DATA_LBA, 2048, false)],
subdirs: Vec::new(),
},
);
// Rewrite the file's ICB: a type-1 hole FIRST, then the real data.
let icb = build_entry_ads(266, 0, 16, &[(1, 2048, HOLE_LBA), (0, 2048, DATA_LBA)], &[]);
disc.put_bytes(PART_START + 12, &icb);
let mut fs = super::read_filesystem(&mut disc).expect("volume mounts");
let lba = fs
.file_start_lba(&mut disc, "/VIDEO_TS.IFO")
.expect("a file whose first descriptor is a hole still has data");
assert_eq!(
lba,
PART_START + DATA_LBA,
"must report the first RECORDED extent; reporting the hole's LBA \\
({}) would rebase every VTS VOB extent onto unrelated sectors",
PART_START + HOLE_LBA
);
}
#[test] #[test]
fn icb_extents_reads_both_entry_types_at_their_own_descriptor_offsets() { fn icb_extents_reads_both_entry_types_at_their_own_descriptor_offsets() {
// ECMA-167 4/14.9 puts a File Entry's L_EA at byte 168 and its // ECMA-167 4/14.9 puts a File Entry's L_EA at byte 168 and its