From 9e59499c56535a8d6252ee06b26658e57374785d Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:13:11 -0500 Subject: [PATCH 01/20] conformance: fix three reference-probe artefacts ref.py and compare.py reported 13 files as mismatches or our-errors that were artefacts of the harness, not differences between the readers: - User-defined links (tall.h5, tudlink.h5, twithub*.h5, tmany.h5, ...): h5py's `get(name, getlink=True)` reports a user-defined link as a HardLink, so ref.py listed it as an object. Read the link type from H5Lget_info instead. - Objects h5py cannot open (cve-2019-8397/8398, cve-2021-46243, cve-2024-32618): the probe deduplicates by header address, ref.py by ObjectID, which an unopenable object does not have, so each extra hard link to it was listed again. Deduplicate those by link address. - Nested array types (tarray3.h5): h5py expands them into trailing dims; hash_values stripped one level and numpy broadcast every element into a whole subarray. Strip every level. compare.py no longer compares the attributes or links of an object h5py could not open at all (cve-2018-17438/17439, cve-2019-9151): h5py read none, so ours are neither extra nor errors against it. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/compare.py | 8 +++++++- conformance/ref.py | 34 ++++++++++++++++++++++++---------- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/conformance/compare.py b/conformance/compare.py index 8c14353..c1d65b2 100755 --- a/conformance/compare.py +++ b/conformance/compare.py @@ -146,7 +146,13 @@ for rel in files: if a.get("kind") != b.get("kind") and "error" not in b and "error" not in a: issues.append(("mismatch", f"{p}: kind {a.get('kind')} vs ours {b.get('kind')}", "kind", b)) ok = False + # h5py could not open the object at all: it read none of its + # attributes or links, so there is nothing to compare ours with + # (the object's own error is compared above and below). + ref_unopened = a.get("kind") == "unknown" and "error" in a for k in ("error", "list_error", "attrs_error"): + if ref_unopened and k != "error": + continue if k in b and k not in a: issues.append(("our-error", f"{p}: {k}: {b[k]}", b[k], b)) ok = False @@ -164,7 +170,7 @@ for rel in files: issues.append(("mismatch", f"{p}: values differ (h5py {a.get('dtype')} vs ours {b.get('dtype')})", "values", b | {"ref_head": a.get("head"), "ref_dtype": a.get("dtype")})) ok = False ra, oa = a.get("attrs") or {}, b.get("attrs") or {} - if "attrs_error" not in b and "attrs_error" not in a: + if "attrs_error" not in b and "attrs_error" not in a and not ref_unopened: for an in sorted(set(ra) | set(oa)): x, y = ra.get(an), oa.get(an) if x is None: diff --git a/conformance/ref.py b/conformance/ref.py index 573deeb..428b6a5 100755 --- a/conformance/ref.py +++ b/conformance/ref.py @@ -111,8 +111,11 @@ def note_conversion(tid, dt, rec): def hash_values(arr, dt, rec): - if dt.subdtype is not None: - # h5py expands an HDF5 array element type into trailing array dims + # h5py expands an HDF5 array element type into trailing array dims, a + # nested array type (an array of arrays) into all of them. Converting the + # expanded array back to the inner subarray type would broadcast every + # element into a whole subarray, so strip every level. + while dt.subdtype is not None: dt = dt.subdtype[0] arr = np.asarray(arr, dtype=dt) if simple(dt): @@ -173,9 +176,13 @@ def main(path): return objects = [] seen = set() - stack = [("/", None)] + # Objects h5py cannot open have no ObjectID to deduplicate by; they are + # deduplicated by the address their hard link points at instead, as the + # probe deduplicates every object by header address. + seen_unopenable = set() + stack = [("/", None, None)] while stack: - p, obj = stack.pop() + p, obj, link_addr = stack.pop() if len(objects) >= MAX_OBJECTS: top["truncated"] = True break @@ -185,6 +192,10 @@ def main(path): obj = f[p] key = hash(obj.id) # h5py ObjectID hash = (fileno, object address/token) except Exception as e: # noqa: BLE001 + if link_addr is not None: + if link_addr in seen_unopenable: + continue + seen_unopenable.add(link_addr) rec["kind"] = "unknown" rec["error"] = err(e) objects.append(rec) @@ -232,15 +243,18 @@ def main(path): base = "" if p == "/" else p kids = [] for n in names: + # The link's own type: `obj.get(n, getlink=True)` reports + # a user-defined link (type 64-255) as a HardLink. try: - link = obj.get(n, getlink=True) + info = obj.id.links.get_info(n.encode("utf-8", "surrogateescape")) except Exception: # noqa: BLE001 - link = None - if link is not None and not isinstance(link, h5py.HardLink): + info = None + if info is not None and info.type != h5py.h5l.TYPE_HARD: continue - kids.append(f"{base}/{n}") - for k in reversed(kids): - stack.append((k, None)) + addr = info.u if info is not None else None + kids.append((f"{base}/{n}", addr)) + for k, addr in reversed(kids): + stack.append((k, None, addr)) except Exception as e: # noqa: BLE001 rec["list_error"] = err(e) objects.append(rec) From 1207df51895c311261e7ceba6413e91e3099fb26 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:14:31 -0500 Subject: [PATCH 02/20] feat(format): ObjectHeader::object_class, libhdf5's object classification libhdf5 decides what an object header is in a fixed order (H5O__obj_class_real): a group if it has a Symbol Table or Link Info message, a dataset if it has a Datatype *and* a Dataspace message, a named datatype if it has a Datatype message. The conformance probe called any header with a Data Layout message a dataset, so cve-2024-33874's /Dset1 (a datatype and a layout, no dataspace), which h5py opens as a named datatype, was reported as a dataset we failed to read (MissingMessage(Dataspace)). The probe now classifies with object_class(). Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 70 +++++++++++++----- crates/clawhdf5-format/src/object_header.rs | 81 +++++++++++++++++++++ 2 files changed, 133 insertions(+), 18 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 72f02a1..aa3d6a1 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -31,7 +31,7 @@ use clawhdf5_format::filter_pipeline::FilterPipeline; use clawhdf5_format::group_v1::{self, GroupEntry}; use clawhdf5_format::group_v2; use clawhdf5_format::message_type::MessageType; -use clawhdf5_format::object_header::ObjectHeader; +use clawhdf5_format::object_header::{ObjectClass, ObjectHeader}; use clawhdf5_format::signature; use clawhdf5_format::superblock::Superblock; use clawhdf5_format::symbol_table::SymbolTableMessage; @@ -657,6 +657,20 @@ fn is_group(h: &ObjectHeader) -> bool { }) } +/// The probe's kind for an object header: libhdf5's object class +/// ([`ObjectHeader::object_class`]: group, then dataset — a datatype *and* a +/// dataspace — then named datatype), which is what h5py opens the object as. +/// The root group, and a header with only link messages, count as groups. +fn kind_of(h: &ObjectHeader, is_root: bool) -> &'static str { + match h.object_class() { + Some(ObjectClass::Group) => "group", + Some(ObjectClass::Dataset) => "dataset", + _ if is_root || is_group(h) => "group", + Some(ObjectClass::NamedDatatype) => "datatype", + None => "unknown", + } +} + fn main() { install_hook(); let path = std::env::args().nth(1).expect("usage: probe "); @@ -735,23 +749,7 @@ fn main() { continue; } }; - let is_ds = h - .messages - .iter() - .any(|m| m.msg_type == MessageType::DataLayout); - let kind = if is_ds { - "dataset" - } else if is_group(&h) || addr == sb.root_group_address { - "group" - } else if h - .messages - .iter() - .any(|m| m.msg_type == MessageType::Datatype) - { - "datatype" - } else { - "unknown" - }; + let kind = kind_of(&h, addr == sb.root_group_address); rec.insert("kind".into(), Value::String(kind.into())); if kind == "dataset" && let Err(msg) = guarded(|| ctx.read_dataset(&h, &mut rec)) @@ -867,6 +865,42 @@ mod tests { assert!(ieee_layout(&f32le)); } + #[test] + fn kind_follows_libhdf5_object_class() { + use clawhdf5_format::object_header::HeaderMessage; + let header = |types: &[MessageType]| ObjectHeader { + version: 2, + messages: types + .iter() + .map(|&msg_type| HeaderMessage { + msg_type, + size: 0, + flags: 0, + creation_order: None, + data: Vec::new(), + }) + .collect(), + reference_count: None, + flags: 0, + access_time: None, + modification_time: None, + change_time: None, + birth_time: None, + }; + use MessageType::*; + // cve-2024-33874 `/Dset1`: a datatype and a layout but no dataspace + // is a named datatype to libhdf5 (h5py opens it as one). + assert_eq!(kind_of(&header(&[Datatype, DataLayout]), false), "datatype"); + assert_eq!( + kind_of(&header(&[Datatype, Dataspace, DataLayout]), false), + "dataset" + ); + assert_eq!(kind_of(&header(&[SymbolTable]), false), "group"); + assert_eq!(kind_of(&header(&[Link]), false), "group"); + assert_eq!(kind_of(&header(&[]), true), "group"); + assert_eq!(kind_of(&header(&[]), false), "unknown"); + } + #[test] fn partial_precision_int_is_shifted_and_sign_extended() { let dt = Datatype::FixedPoint { diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 2c3c819..1fc8696 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -75,7 +75,40 @@ fn read_offset(data: &[u8], pos: usize, size: u8) -> Result { }) } +/// The kind of object an object header describes. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ObjectClass { + /// A group: the header has a Symbol Table or a Link Info message. + Group, + /// A dataset: the header has a Datatype and a Dataspace message. + Dataset, + /// A committed (named) datatype: a Datatype message, no Dataspace. + NamedDatatype, +} + impl ObjectHeader { + /// The kind of object this header describes, decided as libhdf5 decides + /// it (`H5O__obj_class_real`): group first (a Symbol Table or Link Info + /// message), then dataset (a Datatype *and* a Dataspace message — not a + /// Data Layout message), then named datatype (a Datatype message). + /// `None` when none applies; libhdf5 then cannot open the object + /// ("unable to determine object type"). + /// + /// A header with a Datatype and a Data Layout message but no Dataspace + /// is a named datatype to libhdf5, not a dataset. + pub fn object_class(&self) -> Option { + let has = |t: MessageType| self.messages.iter().any(|m| m.msg_type == t); + if has(MessageType::SymbolTable) || has(MessageType::LinkInfo) { + Some(ObjectClass::Group) + } else if has(MessageType::Datatype) && has(MessageType::Dataspace) { + Some(ObjectClass::Dataset) + } else if has(MessageType::Datatype) { + Some(ObjectClass::NamedDatatype) + } else { + None + } + } + /// Parse an object header at the given offset in the data buffer. /// /// `offset_size` and `length_size` come from the superblock. @@ -662,6 +695,54 @@ fn check_message( mod tests { use super::*; + fn header_with(types: &[MessageType]) -> ObjectHeader { + ObjectHeader { + version: 2, + messages: types + .iter() + .map(|&msg_type| HeaderMessage { + msg_type, + size: 0, + flags: 0, + creation_order: None, + data: Vec::new(), + }) + .collect(), + reference_count: None, + flags: 0, + access_time: None, + modification_time: None, + change_time: None, + birth_time: None, + } + } + + #[test] + fn object_class_follows_libhdf5() { + use MessageType::*; + let class = |t: &[MessageType]| header_with(t).object_class(); + assert_eq!( + class(&[Datatype, Dataspace, DataLayout]), + Some(ObjectClass::Dataset) + ); + // A Data Layout message does not make a dataset without a dataspace + // (cve-2024-33874 `/Dset1`: h5py opens it as a named datatype). + assert_eq!( + class(&[Datatype, DataLayout]), + Some(ObjectClass::NamedDatatype) + ); + assert_eq!(class(&[Datatype]), Some(ObjectClass::NamedDatatype)); + // Group messages win over dataset messages. + assert_eq!( + class(&[Datatype, Dataspace, SymbolTable]), + Some(ObjectClass::Group) + ); + assert_eq!(class(&[LinkInfo]), Some(ObjectClass::Group)); + // Link messages alone are not a group; nothing is not an object. + assert_eq!(class(&[Link]), None); + assert_eq!(class(&[]), None); + } + // Helper: build a v1 object header with given messages fn build_v1_header( messages: &[(u16, &[u8], u8)], // (type, data, flags) From 16b7359485713a00ad54f5478941f7f06031aa85 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:15:50 -0500 Subject: [PATCH 03/20] fix(format): a v1 group with an empty link name fails its listing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit libhdf5 refuses to list a symbol-table group that has an entry with an empty name (H5G__ent_to_link: "invalid link name"), so h5py cannot list cve-2021-46244's /BAG_root. We listed it, with an object at "/BAG_root/" (the empty name, pointing at address 0). resolve_v1_group_entries — the listing — now fails with the new FormatError::InvalidLinkName; path lookups still find the group's other names, as libhdf5's by-name lookup does. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/error.rs | 6 ++++ crates/clawhdf5-format/src/group_v1.rs | 44 ++++++++++++++++++++++++-- crates/clawhdf5-format/src/group_v2.rs | 4 ++- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index 0056b14..d285cbf 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -223,6 +223,9 @@ pub enum FormatError { /// The file's actual length in bytes. actual_len: u64, }, + /// A link libhdf5 refuses to list: a symbol-table entry with an empty + /// name ("invalid link name"). Listing the group fails, as in libhdf5. + InvalidLinkName, } impl fmt::Display for FormatError { @@ -494,6 +497,9 @@ impl fmt::Display for FormatError { but the file is {actual_len} bytes" ) } + FormatError::InvalidLinkName => { + write!(f, "invalid link name: a group entry has an empty name") + } } } } diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index 81c149c..a4f97c8 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -21,12 +21,35 @@ pub struct GroupEntry { pub cache_type: u32, } -/// Given a SymbolTableMessage, resolve all group children. +/// Given a SymbolTableMessage, resolve all group children: the group's +/// listing. +/// +/// An entry with an empty name fails the listing with +/// [`FormatError::InvalidLinkName`], as it fails libhdf5's link iteration +/// (`H5G__ent_to_link`: "invalid link name"). Looking a name up +/// ([`resolve_path`], and the path resolution in +/// [`crate::group_v2::resolve_path_any`]) still works in such a group, as it +/// does in libhdf5. pub fn resolve_v1_group_entries( file_data: &[u8], sym_table_msg: &SymbolTableMessage, offset_size: u8, length_size: u8, +) -> Result, FormatError> { + let entries = v1_group_entries(file_data, sym_table_msg, offset_size, length_size)?; + if entries.iter().any(|e| e.name.is_empty()) { + return Err(FormatError::InvalidLinkName); + } + Ok(entries) +} + +/// Every entry of a v1 group, empty names included — for looking a name up, +/// which never matches an empty name. +pub(crate) fn v1_group_entries( + file_data: &[u8], + sym_table_msg: &SymbolTableMessage, + offset_size: u8, + length_size: u8, ) -> Result, FormatError> { // Parse local heap let heap = LocalHeap::parse( @@ -207,8 +230,7 @@ pub fn resolve_path( let mut current_sym_table = root_sym_table.clone(); for (i, component) in components.iter().enumerate() { - let entries = - resolve_v1_group_entries(file_data, ¤t_sym_table, offset_size, length_size)?; + let entries = v1_group_entries(file_data, ¤t_sym_table, offset_size, length_size)?; let found = entries.iter().find(|e| e.name == *component); match found { @@ -425,6 +447,22 @@ mod tests { assert_eq!(entries[1].object_header_address, 0x2000); } + /// cve-2021-46244 `/BAG_root`: a symbol-table entry with an empty name. + /// libhdf5 fails the group's listing ("invalid link name"); a lookup of + /// the other names still works. + #[test] + fn empty_entry_name_fails_the_listing_not_a_lookup() { + let (file, msg) = build_synthetic_group(&[("", 0x1000, 0), ("elevation", 0x2000, 0)], 8, 8); + assert_eq!( + resolve_v1_group_entries(&file, &msg, 8, 8).unwrap_err(), + FormatError::InvalidLinkName + ); + assert_eq!( + resolve_path(&file, &msg, "elevation", 8, 8).unwrap(), + 0x2000 + ); + } + #[test] fn resolve_path_single_level() { let (file, msg) = diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index 57119c9..b645bd6 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -450,7 +450,9 @@ fn resolve_group_entries( .find(|m| m.msg_type == MessageType::SymbolTable) .ok_or_else(|| FormatError::PathNotFound(String::from("no symbol table message")))?; let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; - group_v1::resolve_v1_group_entries(file_data, &stm, offset_size, length_size) + // A lookup: an entry with an empty name (which fails a listing) is + // skipped by the name comparison, as in libhdf5. + group_v1::v1_group_entries(file_data, &stm, offset_size, length_size) } else if is_v2_group(object_header) { resolve_v2_group_entries(file_data, object_header, offset_size, length_size) } else { From 3aab433edb0e857abfd667c82eed2fff95986ad4 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:17:15 -0500 Subject: [PATCH 04/20] fix(format): dataspaces and contiguous storage as libhdf5 reads them - A simple dataspace of rank 0 holds one element in libhdf5 (the product of no dimensions; h5py reads it as shape ()). num_elements() said 0, so cve-2020-18494's /dset1 failed with DataSizeMismatch { expected: 0 }. - A contiguous dataset whose storage is larger than its elements reads: libhdf5 reads the elements from the start of the storage and ignores the rest (H5D__contig_check checks only that they fit in the file). We required the sizes to be equal, so the scalar /Dset1 of cve-2024-32623 and cve-2025-2309 (240 bytes of storage for one int) failed. Storage too small for the elements is still an error. data_read::contiguous_read_len is the rule, used by every contiguous read path. - Dataspace::parse refuses what H5O__sdspace_decode refuses: more than 32 dimensions, a rank on a scalar or null dataspace, a dimension larger than its maximum (new FormatError::InvalidDataspace). Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/data_read.rs | 62 ++++++++++++++----- crates/clawhdf5-format/src/dataspace.rs | 82 ++++++++++++++++++++----- crates/clawhdf5-format/src/error.rs | 7 +++ crates/clawhdf5/src/mmap_file.rs | 8 +-- 4 files changed, 124 insertions(+), 35 deletions(-) diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index 8aec190..d1766a9 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -32,6 +32,22 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr Ok(()) } +/// How many bytes to read from a contiguous dataset's storage of +/// `storage_size` bytes (the layout message's size) holding `needed` bytes +/// of elements. libhdf5 reads the elements' bytes from the start of the +/// storage and ignores storage past them (`H5D__contig_check` checks only +/// that the elements fit in the file), so a larger storage reads; one too +/// small to hold the elements is an error. +pub fn contiguous_read_len(storage_size: u64, needed: usize) -> Result { + if storage_size < needed as u64 { + return Err(FormatError::DataSizeMismatch { + expected: needed, + actual: usize::try_from(storage_size).unwrap_or(usize::MAX), + }); + } + Ok(needed) +} + /// Zero-copy read of contiguous raw data, returning a borrowed slice. /// /// For contiguous layouts, returns a direct `&[u8]` slice into `file_data`. @@ -55,13 +71,7 @@ pub fn read_raw_data_zerocopy<'a>( DataLayout::Contiguous { address, size } => { let addr = address.ok_or(FormatError::NoDataAllocated)?; let addr = addr as usize; - let sz = *size as usize; - if sz != expected_size { - return Err(FormatError::DataSizeMismatch { - expected: expected_size, - actual: sz, - }); - } + let sz = contiguous_read_len(*size, expected_size)?; ensure_len(file_data, addr, sz)?; Ok(Some(&file_data[addr..addr + sz])) } @@ -172,13 +182,7 @@ fn read_raw_data_full_impl( DataLayout::Contiguous { address, size } => { let addr = address.ok_or(FormatError::NoDataAllocated)?; let addr = addr as usize; - let sz = *size as usize; - if sz != expected_size { - return Err(FormatError::DataSizeMismatch { - expected: expected_size, - actual: sz, - }); - } + let sz = contiguous_read_len(*size, expected_size)?; ensure_len(file_data, addr, sz)?; let mut out = crate::bulk_alloc::vec_for_bulk(sz); out.extend_from_slice(&file_data[addr..addr + sz]); @@ -2632,6 +2636,36 @@ mod tests { assert_eq!(result.unwrap(), &[1.5f32, 2.5, 3.5]); } + /// libhdf5 reads a contiguous dataset's elements from the start of its + /// storage and ignores storage past them (cve-2024-32623's scalar + /// `/Dset1` has 240 bytes of storage for one 4-byte element). Storage too + /// small for the elements is still an error. + #[test] + fn contiguous_storage_larger_than_the_elements_reads() { + let dt = make_f64_le_type(); + let ds = make_simple_dataspace(&[2]); + let mut file_data = vec![0u8; 64]; + file_data[..8].copy_from_slice(&1.5f64.to_le_bytes()); + file_data[8..16].copy_from_slice(&2.5f64.to_le_bytes()); + file_data[16..24].copy_from_slice(&9.0f64.to_le_bytes()); + let layout = DataLayout::Contiguous { + address: Some(0), + size: 40, + }; + let raw = read_raw_data(&file_data, &layout, &ds, &dt).unwrap(); + assert_eq!(raw, file_data[..16]); + let zc = read_raw_data_zerocopy(&file_data, &layout, &ds, &dt).unwrap(); + assert_eq!(zc, Some(&file_data[..16])); + let small = DataLayout::Contiguous { + address: Some(0), + size: 8, + }; + assert!(matches!( + read_raw_data(&file_data, &small, &ds, &dt), + Err(FormatError::DataSizeMismatch { .. }) + )); + } + #[test] fn zerocopy_size_mismatch() { let dt = make_f64_le_type(); diff --git a/crates/clawhdf5-format/src/dataspace.rs b/crates/clawhdf5-format/src/dataspace.rs index a08ca9d..8eb37e1 100644 --- a/crates/clawhdf5-format/src/dataspace.rs +++ b/crates/clawhdf5-format/src/dataspace.rs @@ -7,6 +7,9 @@ use alloc::vec::Vec; use crate::error::FormatError; +/// Most dimensions a dataspace can have (`H5S_MAX_RANK`). +pub const MAX_RANK: u8 = 32; + /// Type of dataspace. #[derive(Debug, Clone, PartialEq)] pub enum DataspaceType { @@ -67,6 +70,12 @@ impl Dataspace { let version = data[0]; let rank = data[1]; let flags = data[2]; + // H5O__sdspace_decode's checks. + if rank > MAX_RANK { + return Err(FormatError::InvalidDataspace( + "simple dataspace dimensionality is too large", + )); + } let (space_type, header_size) = match version { 1 => { @@ -88,6 +97,11 @@ impl Dataspace { 2 => DataspaceType::Null, _ => return Err(FormatError::InvalidDataspaceType(type_byte)), }; + if st != DataspaceType::Simple && rank > 0 { + return Err(FormatError::InvalidDataspace( + "invalid rank for scalar or NULL dataspace", + )); + } (st, 4usize) } _ => return Err(FormatError::InvalidDataspaceVersion(version)), @@ -107,8 +121,13 @@ impl Dataspace { // Read max dimensions if flags bit 0 is set let max_dimensions = if flags & 0x01 != 0 { let mut max_dims = Vec::with_capacity(rank as usize); - for _ in 0..rank { + for i in 0..rank as usize { let val = read_length(data, pos, length_size)?; + if dimensions[i] > val { + return Err(FormatError::InvalidDataspace( + "dataspace dimension size is greater than its maximum size", + )); + } max_dims.push(val); pos += ls; } @@ -176,7 +195,6 @@ impl Dataspace { match self.space_type { DataspaceType::Null => Ok(0), DataspaceType::Scalar => Ok(1), - DataspaceType::Simple if self.dimensions.is_empty() => Ok(0), DataspaceType::Simple => self .dimensions .iter() @@ -195,18 +213,14 @@ impl Dataspace { match self.space_type { DataspaceType::Null => 0, DataspaceType::Scalar => 1, - DataspaceType::Simple => { - if self.dimensions.is_empty() { - 0 - } else { - // Saturate rather than wrap: a wrapped product could - // under-size a buffer. Size-critical callers use - // `checked_num_elements`. - self.dimensions - .iter() - .fold(1u64, |acc, &d| acc.saturating_mul(d)) - } - } + // A simple dataspace of rank 0 holds one element, as in libhdf5 + // (the product of no dimensions). Saturate rather than wrap: a + // wrapped product could under-size a buffer. Size-critical + // callers use `checked_num_elements`. + DataspaceType::Simple => self + .dimensions + .iter() + .fold(1u64, |acc, &d| acc.saturating_mul(d)), } } } @@ -352,4 +366,44 @@ mod tests { let ds = Dataspace::parse(&data, 8).unwrap(); assert_eq!(ds.max_dimensions, Some(vec![10])); } + + /// A simple dataspace of rank 0 (cve-2020-18494's `/dset1`) holds one + /// element in libhdf5, which h5py reads as shape `()`. It was 0. + #[test] + fn simple_rank_zero_holds_one_element() { + let data = build_v2_dataspace(0, 0, 1, &[], None); + let ds = Dataspace::parse(&data, 8).unwrap(); + assert_eq!(ds.space_type, DataspaceType::Simple); + assert_eq!(ds.num_elements(), 1); + assert_eq!(ds.checked_num_elements().unwrap(), 1); + } + + /// `H5O__sdspace_decode`'s checks. + #[test] + fn refuses_what_libhdf5_refuses() { + let too_many = build_v2_dataspace(33, 0, 1, &[1; 33], None); + assert!(matches!( + Dataspace::parse(&too_many, 8), + Err(FormatError::InvalidDataspace(_)) + )); + let scalar_with_rank = build_v2_dataspace(1, 0, 0, &[4], None); + assert!(matches!( + Dataspace::parse(&scalar_with_rank, 8), + Err(FormatError::InvalidDataspace(_)) + )); + let null_with_rank = build_v2_dataspace(1, 0, 2, &[4], None); + assert!(matches!( + Dataspace::parse(&null_with_rank, 8), + Err(FormatError::InvalidDataspace(_)) + )); + let over_max = build_v1_dataspace(2, 0x01, &[5, 20], Some(&[10, 10])); + assert!(matches!( + Dataspace::parse(&over_max, 8), + Err(FormatError::InvalidDataspace(_)) + )); + // 32 dimensions, and a size equal to the maximum or unlimited, are fine. + assert!(Dataspace::parse(&build_v2_dataspace(32, 0, 1, &[1; 32], None), 8).is_ok()); + let at_max = build_v1_dataspace(2, 0x01, &[10, 20], Some(&[10, u64::MAX])); + assert!(Dataspace::parse(&at_max, 8).is_ok()); + } } diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index d285cbf..b365794 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -226,6 +226,10 @@ pub enum FormatError { /// A link libhdf5 refuses to list: a symbol-table entry with an empty /// name ("invalid link name"). Listing the group fails, as in libhdf5. InvalidLinkName, + /// A dataspace message libhdf5 refuses to decode (the reason is + /// libhdf5's own error text): more than 32 dimensions, a rank on a + /// scalar or null dataspace, a dimension larger than its maximum. + InvalidDataspace(&'static str), } impl fmt::Display for FormatError { @@ -500,6 +504,9 @@ impl fmt::Display for FormatError { FormatError::InvalidLinkName => { write!(f, "invalid link name: a group entry has an empty name") } + FormatError::InvalidDataspace(why) => { + write!(f, "invalid dataspace: {why}") + } } } } diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 76d9ea1..59adee9 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -390,13 +390,7 @@ impl<'f> MmapDataset<'f> { match &dl { DataLayout::Contiguous { address, size } => { let addr = address.ok_or(Error::Format(FormatError::NoDataAllocated))?; - let sz = *size as usize; - if sz != expected { - return Err(Error::Format(FormatError::DataSizeMismatch { - expected, - actual: sz, - })); - } + let sz = clawhdf5_format::data_read::contiguous_read_len(*size, expected)?; let data = self.file.hdf5_bytes(); let a = addr as usize; if a + sz > data.len() { From b22b15f00a868e3e4fa48c70ebc636eaac3944b5 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:22:04 -0500 Subject: [PATCH 05/20] fix: refuse at open the dataset storage libhdf5 refuses at open libhdf5 checks a dataset's storage when it opens the dataset (H5D__contig_check, H5D__compact_init): the element count times the element size must not overflow, contiguous storage must end inside the file, compact data must be the dataset's size. File::dataset opened cve-2024-32624's /Dset_OBJREF (2^62 + 2 references of 8 bytes) and reported its shape; only reading failed. data_read::check_dataset_storage makes those checks (new FormatError::InvalidDatasetStorage), and File, MmapFile and LazyFile run it whenever they open a dataset (by path, by address, from a group), as does the conformance probe. As before, a datatype, dataspace or layout that does not decode is left for the read to report, so such a dataset still opens and its attributes still read. An empty contiguous dataset at a defined address, which libhdf5 refuses, is still accepted: clawhdf5 up to v2.7.0 wrote them. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 14 +-- crates/clawhdf5-format/src/data_read.rs | 93 +++++++++++++++++++ crates/clawhdf5-format/src/error.rs | 8 ++ crates/clawhdf5/src/lazy.rs | 28 +++++- crates/clawhdf5/src/mmap_file.rs | 28 +++++- crates/clawhdf5/src/reader.rs | 33 +++++-- .../tests/header_validation_interop.rs | 49 ++++++++++ 7 files changed, 233 insertions(+), 20 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index aa3d6a1..844f343 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -308,18 +308,20 @@ impl<'a> Ctx<'a> { ) .map_err(e)?; } - let (shape, n) = Self::shape(&ds); - rec.insert("shape".into(), shape); - if n.saturating_mul(dt.type_size() as u64) > MAX_BYTES { - rec.insert("skipped".into(), Value::String("too large".into())); - return Ok(()); - } let lm = h .messages .iter() .find(|m| m.msg_type == MessageType::DataLayout) .ok_or("MissingMessage(DataLayout)")?; let dl = DataLayout::parse(&lm.data, self.os, self.ls).map_err(e)?; + // What libhdf5 checks when it opens the dataset (as File::dataset). + data_read::check_dataset_storage(&dl, &ds, &dt, self.data.len() as u64).map_err(e)?; + let (shape, n) = Self::shape(&ds); + rec.insert("shape".into(), shape); + if n.saturating_mul(dt.type_size() as u64) > MAX_BYTES { + rec.insert("skipped".into(), Value::String("too large".into())); + return Ok(()); + } rec.insert( "layout".into(), Value::String( diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index d1766a9..591142c 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -32,6 +32,64 @@ fn ensure_len(data: &[u8], offset: usize, needed: usize) -> Result<(), FormatErr Ok(()) } +/// The storage checks libhdf5 makes when it opens a dataset, before any +/// data is read (`H5D__contig_check`, `H5D__compact_init`), so a dataset +/// they refuse fails to open, as in libhdf5, instead of opening and +/// reporting a shape nothing can be read from: +/// +/// - the element count times the element size must not overflow 64 bits +/// ("size of dataset's storage overflowed" — `cve-2024-32624` +/// `/Dset_OBJREF`, 2^62 references of 8 bytes); +/// - contiguous storage at a defined address must end within the file's +/// `file_len` bytes (the HDF5 data up to the end of file the superblock +/// records); +/// - compact data must be exactly the dataset's size. +/// +/// Deliberately not refused, unlike libhdf5: an empty contiguous dataset at +/// a defined address (libhdf5's overflow test `addr + 0 <= addr` refuses +/// it), which clawhdf5 up to v2.7.0 wrote. Chunked and virtual layouts are +/// checked when their data is read. +pub fn check_dataset_storage( + layout: &DataLayout, + dataspace: &Dataspace, + datatype: &Datatype, + file_len: u64, +) -> Result<(), FormatError> { + if !matches!( + layout, + DataLayout::Contiguous { .. } | DataLayout::Compact { .. } + ) { + return Ok(()); + } + const OVERFLOWED: &str = "size of dataset's storage overflowed"; + let n = dataspace + .checked_num_elements() + .map_err(|_| FormatError::InvalidDatasetStorage(OVERFLOWED))?; + let data_size = n + .checked_mul(u64::from(datatype.type_size())) + .ok_or(FormatError::InvalidDatasetStorage(OVERFLOWED))?; + match layout { + DataLayout::Contiguous { + address: Some(address), + .. + } if address + .checked_add(data_size) + .is_none_or(|end| end > file_len) => + { + Err(FormatError::InvalidDatasetStorage( + "invalid dataset size, likely file corruption", + )) + } + DataLayout::Compact { data } if data.len() as u64 != data_size => { + Err(FormatError::InvalidDatasetStorage( + "bad value from dataset header - size of compact dataset's data buffer \ + doesn't match size of dataset data", + )) + } + _ => Ok(()), + } +} + /// How many bytes to read from a contiguous dataset's storage of /// `storage_size` bytes (the layout message's size) holding `needed` bytes /// of elements. libhdf5 reads the elements' bytes from the start of the @@ -2666,6 +2724,41 @@ mod tests { )); } + /// `H5D__contig_check` / `H5D__compact_init`, run when a dataset opens. + #[test] + fn dataset_storage_checks_at_open() { + let dt = make_f64_le_type(); + let contiguous = |address| DataLayout::Contiguous { address, size: 0 }; + // cve-2024-32624 `/Dset_OBJREF`: 2^62 + 2 elements of 8 bytes. + let huge = make_simple_dataspace(&[(1 << 62) + 2]); + assert_eq!( + check_dataset_storage(&contiguous(None), &huge, &dt, 1 << 20), + Err(FormatError::InvalidDatasetStorage( + "size of dataset's storage overflowed" + )) + ); + let ds = make_simple_dataspace(&[4]); + assert!(check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 132).is_ok()); + assert!(matches!( + check_dataset_storage(&contiguous(Some(100)), &ds, &dt, 131), + Err(FormatError::InvalidDatasetStorage(_)) + )); + assert!(matches!( + check_dataset_storage(&contiguous(Some(u64::MAX - 8)), &ds, &dt, u64::MAX), + Err(FormatError::InvalidDatasetStorage(_)) + )); + // Not allocated, and (unlike libhdf5) empty at a defined address. + assert!(check_dataset_storage(&contiguous(None), &ds, &dt, 0).is_ok()); + let empty = make_simple_dataspace(&[0]); + assert!(check_dataset_storage(&contiguous(Some(64)), &empty, &dt, 64).is_ok()); + let compact = |n: usize| DataLayout::Compact { data: vec![0; n] }; + assert!(check_dataset_storage(&compact(32), &ds, &dt, 0).is_ok()); + assert!(matches!( + check_dataset_storage(&compact(24), &ds, &dt, 0), + Err(FormatError::InvalidDatasetStorage(_)) + )); + } + #[test] fn zerocopy_size_mismatch() { let dt = make_f64_le_type(); diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index b365794..ddd9ba7 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -230,6 +230,11 @@ pub enum FormatError { /// libhdf5's own error text): more than 32 dimensions, a rank on a /// scalar or null dataspace, a dimension larger than its maximum. InvalidDataspace(&'static str), + /// A dataset whose storage libhdf5 refuses when it opens the dataset + /// (the reason is libhdf5's own error text): an element count times + /// element size that overflows, contiguous storage past the end of the + /// file, compact data of the wrong size. + InvalidDatasetStorage(&'static str), } impl fmt::Display for FormatError { @@ -507,6 +512,9 @@ impl fmt::Display for FormatError { FormatError::InvalidDataspace(why) => { write!(f, "invalid dataspace: {why}") } + FormatError::InvalidDatasetStorage(why) => { + write!(f, "invalid dataset storage: {why}") + } } } } diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index a33b9bb..3232ee6 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -135,10 +135,11 @@ impl LazyFile { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(LazyDataset { + LazyDataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `LazyGroup` handle. @@ -284,10 +285,11 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(LazyDataset { + LazyDataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -326,6 +328,24 @@ pub struct LazyDataset<'f, R: HDF5Read> { } impl<'f, R: HDF5Read> LazyDataset<'f, R> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.hdf5_bytes().len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 59adee9..2a8400b 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -85,10 +85,11 @@ impl MmapFile { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(MmapDataset { + MmapDataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `MmapGroup` handle. @@ -211,10 +212,11 @@ impl<'f> MmapGroup<'f> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(MmapDataset { + MmapDataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -253,6 +255,24 @@ pub struct MmapDataset<'f> { } impl<'f> MmapDataset<'f> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.hdf5_bytes().len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 90faf11..40ac08b 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -170,10 +170,11 @@ impl File { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(path.to_string())); } - Ok(Dataset { + Dataset { file: self, header: hdr, - }) + } + .check_open() } /// A `Dataset` handle for the object header at `address` (an address @@ -186,10 +187,11 @@ impl File { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(format!("object at address {address}"))); } - Ok(Dataset { + Dataset { file: self, header: hdr, - }) + } + .check_open() } /// Resolve a path and return a `Group` handle. @@ -427,10 +429,11 @@ impl<'f> Group<'f> { if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } - Ok(Dataset { + Dataset { file: self.file, header: hdr, - }) + } + .check_open() } /// Get a subgroup within this group by name. @@ -469,6 +472,24 @@ pub struct Dataset<'f> { } impl<'f> Dataset<'f> { + /// libhdf5's storage checks when it opens a dataset + /// ([`data_read::check_dataset_storage`]): a dataset whose element count + /// times element size overflows, or whose contiguous storage runs past + /// the end of the file, fails to open. A datatype, dataspace or layout + /// that does not decode is left for the read to report, as before (the + /// dataset still opens, and its attributes can be read). + fn check_open(self) -> Result { + let decoded = (|| -> Result<_, Error> { + let data = self.required_payload(MessageType::Dataspace)?; + let ds = Dataspace::parse(&data, self.file.length_size())?; + Ok((self.datatype()?, ds, self.data_layout()?)) + })(); + if let Ok((dt, ds, dl)) = decoded { + data_read::check_dataset_storage(&dl, &ds, &dt, self.file.data.len() as u64)?; + } + Ok(self) + } + /// Returns the shape (dimensions) of the dataset. pub fn shape(&self) -> Result, Error> { let ds = self.dataspace()?; diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 2f52d27..0bfd30e 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -508,3 +508,52 @@ for libver in ("earliest", "latest"): ); } } + +/// cve-2024-32624 `/Dset_OBJREF`: a dataspace whose element count times the +/// element size overflows 64 bits. libhdf5 refuses to open the dataset +/// ("size of dataset's storage overflowed"); `File::dataset` used to open it +/// and report its shape, and only reading failed. The same for storage that +/// runs past the end of the file ("invalid dataset size, likely file +/// corruption"). +#[test] +fn dataset_storage_libhdf5_refuses_at_open_is_refused_at_open() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + // A contiguous int64 dataset of 3 elements, libver earliest (a version 1 + // dataspace holding the size and the maximum size). "overflow" makes + // both 2^62 + 2 (x 8 bytes overflows); "pasteof" makes both 1000 (the + // storage runs past the end of the file). + let verdicts = h5py_verdicts( + dir.path(), + r#" +good = os.path.join(d, "good.h5") +with h5py.File(good, "w", libver="earliest") as f: + f.create_dataset("d", data=np.array([7, 8, 9], dtype=" 0 and data.find(three, at + 1) < 0 +for name, n in (("overflow", (1 << 62) + 2), ("pasteof", 1000)): + bad = bytearray(data) + bad[at:at + 16] = struct.pack(" Date: Sat, 26 Sep 2026 10:28:10 -0500 Subject: [PATCH 06/20] style(format): iterate the dimensions when checking them against their maxima Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/dataspace.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/clawhdf5-format/src/dataspace.rs b/crates/clawhdf5-format/src/dataspace.rs index 8eb37e1..eedba2f 100644 --- a/crates/clawhdf5-format/src/dataspace.rs +++ b/crates/clawhdf5-format/src/dataspace.rs @@ -121,9 +121,9 @@ impl Dataspace { // Read max dimensions if flags bit 0 is set let max_dimensions = if flags & 0x01 != 0 { let mut max_dims = Vec::with_capacity(rank as usize); - for i in 0..rank as usize { + for &dim in &dimensions { let val = read_length(data, pos, length_size)?; - if dimensions[i] > val { + if dim > val { return Err(FormatError::InvalidDataspace( "dataspace dimension size is greater than its maximum size", )); From b3058ca46eb36a4d66090de679dd705e6b61106b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:28:28 -0500 Subject: [PATCH 07/20] feat: decode the superblock extension at open; read metadata cache images libhdf5 decodes the messages of a v2/v3 superblock's extension when it opens a file (H5F__super_read) and refuses the file when one does not decode. We never looked at them, so we opened cve-2020-10810 (a File Space Info message too short for the free-space manager addresses it announces) and cve-2020-10812 (a metadata cache image past the end of the file), both of which libhdf5 refuses. A file written with a metadata cache image keeps its metadata cache entries in an image block the extension points at; libhdf5 loads them over the file's own bytes before it reads any metadata (H5C__load_cache_image, H5C__reconstruct_cache_contents). In h5clear_mdc_image.h5 the root group's header exists only in the image, so every reader failed with InvalidObjectHeaderVersion(0). The new clawhdf5_format::superblock_ext module: - read_superblock_extension decodes the v1 B-tree K, File Space Info and Metadata Cache Image messages with libhdf5's checks (versions, page size 512 B .. 1 GiB, the addresses a persisting message lists, the image inside the file), with the new FormatError::InvalidSuperblockExtension; - apply_cache_image checks an image block as libhdf5 does (signature, version, recorded length, entry types, rings, ages, addresses inside the file and not repeated, flush-dependency parents) and returns the file's bytes with every entry written at its address (FormatError::InvalidCacheImage); - metadata_view does both. File, MmapFile and LazyFile (and so h5rs) call metadata_view at open and read an image file through the patched copy; the conformance probe does the same. The image's trailing checksum is not verified, as libhdf5 does not verify it. tests/fixtures/h5clear_mdc_image.h5 is libhdf5's own test file. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 13 + crates/clawhdf5-format/src/error.rs | 14 + crates/clawhdf5-format/src/lib.rs | 1 + crates/clawhdf5-format/src/superblock_ext.rs | 630 ++++++++++++++++++ crates/clawhdf5/src/lazy.rs | 17 +- crates/clawhdf5/src/mmap_file.rs | 15 +- crates/clawhdf5/src/reader.rs | 25 +- .../tests/fixtures/h5clear_mdc_image.h5 | Bin 0 -> 23467 bytes .../tests/header_validation_interop.rs | 91 ++- crates/clawhdf5/tests/metadata_cache_image.rs | 48 ++ 10 files changed, 839 insertions(+), 15 deletions(-) create mode 100644 crates/clawhdf5-format/src/superblock_ext.rs create mode 100644 crates/clawhdf5/tests/fixtures/h5clear_mdc_image.h5 create mode 100644 crates/clawhdf5/tests/metadata_cache_image.rs diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 844f343..c4cf524 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -712,6 +712,19 @@ fn main() { return; } }; + // libhdf5 decodes the superblock extension at open, and loads a + // metadata cache image over the file's own metadata. + let view = match guarded(|| { + clawhdf5_format::superblock_ext::metadata_view(hdf5, &sb).map_err(e) + }) { + Ok(v) => v, + Err(msg) => { + top.insert("open_error".into(), Value::String(msg)); + println!("{}", Value::Object(top)); + return; + } + }; + let hdf5: &[u8] = view.as_deref().unwrap_or(hdf5); top.insert("superblock_version".into(), json!(sb.version)); let ctx = Ctx { data: hdf5, diff --git a/crates/clawhdf5-format/src/error.rs b/crates/clawhdf5-format/src/error.rs index ddd9ba7..7f9e3d2 100644 --- a/crates/clawhdf5-format/src/error.rs +++ b/crates/clawhdf5-format/src/error.rs @@ -235,6 +235,14 @@ pub enum FormatError { /// element size that overflows, contiguous storage past the end of the /// file, compact data of the wrong size. InvalidDatasetStorage(&'static str), + /// A superblock extension message libhdf5 refuses to decode when it + /// opens the file (the reason is libhdf5's own error text): a File Space + /// Info message that runs off its end or has a bad page size, a metadata + /// cache image outside the file, … + InvalidSuperblockExtension(&'static str), + /// A metadata cache image block libhdf5 refuses to load (the reason is + /// libhdf5's own error text). + InvalidCacheImage(&'static str), } impl fmt::Display for FormatError { @@ -515,6 +523,12 @@ impl fmt::Display for FormatError { FormatError::InvalidDatasetStorage(why) => { write!(f, "invalid dataset storage: {why}") } + FormatError::InvalidSuperblockExtension(why) => { + write!(f, "invalid superblock extension: {why}") + } + FormatError::InvalidCacheImage(why) => { + write!(f, "invalid metadata cache image: {why}") + } } } } diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index 3e30f9d..e10711b 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -118,6 +118,7 @@ pub mod selection; pub mod shared_message; pub mod signature; pub mod superblock; +pub mod superblock_ext; pub mod symbol_table; #[cfg(all( test, diff --git a/crates/clawhdf5-format/src/superblock_ext.rs b/crates/clawhdf5-format/src/superblock_ext.rs new file mode 100644 index 0000000..d7ec3cc --- /dev/null +++ b/crates/clawhdf5-format/src/superblock_ext.rs @@ -0,0 +1,630 @@ +//! The superblock extension of a version 2 or 3 superblock, and the +//! metadata cache image it can point to. +//! +//! libhdf5 reads the extension when it opens a file (`H5F__super_read`) and +//! decodes the messages that configure the file: v1 B-tree "K" values, File +//! Space Info, and the Metadata Cache Image. A message that does not decode +//! makes the file fail to open, so [`read_superblock_extension`] decodes and +//! checks them the way libhdf5 does. +//! +//! A metadata cache image (written with `H5Pset_mdc_image_config`) is a +//! block holding serialized metadata cache entries — object headers, B-tree +//! nodes, heaps — each with its file address. libhdf5 loads it into its +//! cache before it reads any other metadata (`H5C__load_cache_image`, +//! `H5C__reconstruct_cache_contents`), and the entries take the place of +//! the file's bytes at their addresses: the file itself may hold stale or +//! no metadata there (in `h5clear_mdc_image.h5` the root group's header is +//! only in the image). [`apply_cache_image`] does the same with bytes: it +//! returns a copy of the file with every entry written at its address, so +//! every parser reads what libhdf5 reads. + +#[cfg(not(feature = "std"))] +use alloc::{collections::BTreeSet, vec::Vec}; +#[cfg(feature = "std")] +use std::collections::BTreeSet; + +use crate::error::FormatError; +use crate::message_type::MessageType; +use crate::object_header::ObjectHeader; +use crate::superblock::Superblock; + +/// Message type of the File Space Info message. +const MSG_FSINFO: u16 = 0x0017; +/// Message type of the Metadata Cache Image message. +const MSG_MDCI: u16 = 0x0018; +/// Header message flag: the library did not know the message when it wrote +/// it back (`H5O_MSG_FLAG_WAS_UNKNOWN`); libhdf5 then ignores its contents. +const MSG_FLAG_WAS_UNKNOWN: u8 = 0x20; + +/// `H5F_FILE_SPACE_PAGE_SIZE_MIN` / `_MAX`. +const PAGE_SIZE_MIN: u64 = 512; +const PAGE_SIZE_MAX: u64 = 1024 * 1024 * 1024; +/// libhdf5's default file space page size, used for a version 0 message. +const PAGE_SIZE_DEFAULT: u64 = 4096; +/// Free-space managers whose addresses a persisting version 1 File Space +/// Info message lists (`H5F_MEM_PAGE_SUPER` .. `H5F_MEM_PAGE_NTYPES`), and +/// a version 0 one (`H5FD_MEM_SUPER` .. `H5FD_MEM_NTYPES`). +const FSM_ADDRS_V1: usize = 12; +const FSM_ADDRS_V0: usize = 6; + +/// Metadata cache image block limits (`H5Cimage.c`, `H5ACprivate.h`). +const MDCI_SIGNATURE: &[u8; 4] = b"MDCI"; +const MDCI_HAVE_RESIZE_STATUS: u8 = 0x01; +const MDCI_ENTRY_IS_FD_PARENT: u8 = 0x04; +const MDCI_ENTRY_IS_FD_CHILD: u8 = 0x08; +/// `H5AC_NTYPES`: entry type ids are below this. +const MDCI_NTYPES: u8 = 30; +/// `H5C_RING_NTYPES`. +const MDCI_RING_NTYPES: u8 = 6; +/// `H5AC__CACHE_IMAGE__ENTRY_AGEOUT__MAX`. +const MDCI_AGE_MAX: u8 = 100; + +/// A decoded File Space Info message (0x0017), mapped to version 1 as +/// libhdf5 maps a version 0 one. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileSpaceInfo { + /// Message version as stored (0 or 1). + pub version: u8, + /// File space strategy (`H5F_fspace_strategy_t`). + pub strategy: u8, + /// Whether free space is persisted. + pub persist: bool, + /// Free-space section threshold. + pub threshold: u64, + /// File space page size. + pub page_size: u64, +} + +/// Where a metadata cache image block is (Metadata Cache Image message, +/// 0x0018). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct CacheImageLocation { + /// Address of the image block. + pub address: u64, + /// Length of the image block in bytes. + pub length: u64, +} + +/// The messages of a superblock extension that libhdf5 decodes at open. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SuperblockExtension { + /// v1 B-tree "K" values (chunk index, symbol table node, symbol table + /// leaf), when the extension overrides the defaults. + pub btree_k: Option<(u16, u16, u16)>, + /// The File Space Info message. + pub file_space_info: Option, + /// The metadata cache image, when the file has one. + pub cache_image: Option, +} + +fn ext_err(why: &'static str) -> FormatError { + FormatError::InvalidSuperblockExtension(why) +} + +const RAN_OFF: &str = "ran off end of input buffer while decoding"; + +/// A little-endian cursor over one message or block, failing with +/// `overrun` when it runs off the end. +struct Cursor<'a> { + data: &'a [u8], + pos: usize, + overrun: FormatError, +} + +impl<'a> Cursor<'a> { + fn new(data: &'a [u8], overrun: FormatError) -> Self { + Cursor { + data, + pos: 0, + overrun, + } + } + + fn take(&mut self, n: usize) -> Result<&'a [u8], FormatError> { + let end = self + .pos + .checked_add(n) + .filter(|&e| e <= self.data.len()) + .ok_or_else(|| self.overrun.clone())?; + let s = &self.data[self.pos..end]; + self.pos = end; + Ok(s) + } + + fn u8(&mut self) -> Result { + Ok(self.take(1)?[0]) + } + + fn uint(&mut self, width: u8) -> Result { + let b = self.take(width as usize)?; + Ok(b.iter() + .rev() + .fold(0u64, |acc, &x| (acc << 8) | u64::from(x))) + } + + /// An address of `width` bytes; `None` when undefined (all ones). + fn addr(&mut self, width: u8) -> Result, FormatError> { + let v = self.uint(width)?; + let undef = if width >= 8 { + u64::MAX + } else { + (1u64 << (8 * u32::from(width))) - 1 + }; + Ok((v != undef).then_some(v)) + } +} + +/// Decode and check the superblock extension of `sb`, as libhdf5 does when +/// it opens the file. `data` is the file from the superblock on, up to the +/// end of file the superblock records (its end is libhdf5's "eoa"). +/// +/// Returns `Ok(None)` for a superblock without an extension (versions 0 +/// and 1 have none). A message libhdf5 fails to decode, or a cache image +/// that does not lie inside the file, is an error: libhdf5 refuses to open +/// such a file (`cve-2020-10810`: a File Space Info message too short for +/// the free-space manager addresses it announces; `cve-2020-10812`: a cache +/// image past the end of the file). +pub fn read_superblock_extension( + data: &[u8], + sb: &Superblock, +) -> Result, FormatError> { + let os = sb.offset_size; + let ls = sb.length_size; + let undef = if os >= 8 { + u64::MAX + } else { + (1u64 << (8 * u32::from(os))) - 1 + }; + let Some(addr) = sb.superblock_extension_address.filter(|&a| a != undef) else { + return Ok(None); + }; + let addr = usize::try_from(addr).map_err(|_| ext_err("address out of range"))?; + let header = ObjectHeader::parse(data, addr, os, ls)?; + let eoa = data.len() as u64; + + let mut ext = SuperblockExtension::default(); + for msg in &header.messages { + match msg.msg_type { + MessageType::BTreeKValues => { + let mut c = Cursor::new(&msg.data, ext_err(RAN_OFF)); + if c.u8()? != 0 { + return Err(ext_err("bad version number for v1 B-tree 'K' message")); + } + let chunk = c.uint(2)? as u16; + let snode = c.uint(2)? as u16; + let leaf = c.uint(2)? as u16; + ext.btree_k = Some((chunk, snode, leaf)); + } + MessageType::Unknown(MSG_FSINFO) if msg.flags & MSG_FLAG_WAS_UNKNOWN == 0 => { + ext.file_space_info = Some(decode_fsinfo(&msg.data, os, ls)?); + } + MessageType::Unknown(MSG_MDCI) => { + let mut c = Cursor::new(&msg.data, ext_err(RAN_OFF)); + if c.u8()? != 0 { + return Err(ext_err( + "bad version number for metadata cache image message", + )); + } + let address = c.addr(os)?; + let length = c.uint(ls)?; + let Some(address) = address else { + return Err(ext_err("metadata cache image address is undefined")); + }; + if address.checked_add(length).is_none_or(|end| end > eoa) { + return Err(ext_err( + "metadata cache image: address plus size exceeds file eoa", + )); + } + ext.cache_image = Some(CacheImageLocation { address, length }); + } + _ => {} + } + } + Ok(Some(ext)) +} + +/// `H5O__fsinfo_decode` plus the checks `H5F__super_read` makes on it. +fn decode_fsinfo(data: &[u8], os: u8, ls: u8) -> Result { + let mut c = Cursor::new(data, ext_err(RAN_OFF)); + let version = c.u8()?; + let info = if version == 0 { + let old_strategy = c.u8()?; + let threshold = c.uint(ls)?; + // H5F_file_space_type_t: 1 ALL_PERSIST, 2 ALL, 3 AGGR_VFD, 4 VFD. + let (strategy, persist) = match old_strategy { + 1 => { + for _ in 0..FSM_ADDRS_V0 { + c.addr(os)?; + } + (0, true) + } + 2 => (0, false), + 3 => (2, false), + 4 => (3, false), + _ => return Err(ext_err("invalid file space strategy")), + }; + FileSpaceInfo { + version, + strategy, + persist, + threshold, + page_size: PAGE_SIZE_DEFAULT, + } + } else { + if version > 1 { + return Err(ext_err("File space info message's version out of bounds")); + } + let strategy = c.u8()?; + let persist = c.u8()? != 0; + let threshold = c.uint(ls)?; + let page_size = c.uint(ls)?; + if page_size == 0 || page_size > PAGE_SIZE_MAX { + return Err(ext_err("invalid page size in file space info")); + } + c.uint(2)?; // page end metadata threshold + c.addr(os)?; // EOA before the free-space managers + if persist { + for _ in 0..FSM_ADDRS_V1 { + c.addr(os)?; + } + } + FileSpaceInfo { + version, + strategy, + persist, + threshold, + page_size, + } + }; + if info.page_size < PAGE_SIZE_MIN { + return Err(ext_err("file space page size too small")); + } + Ok(info) +} + +/// One entry of a metadata cache image: `len` bytes at `image_offset` in +/// the image block, belonging at file address `address`. +struct ImageEntry { + address: u64, + image_offset: usize, + len: usize, +} + +/// Decode the metadata cache image at `location` in `data` (the file from +/// the superblock on, up to its recorded end of file) and return a copy of +/// `data` with every cached entry written at its address — the metadata +/// libhdf5 reads for this file. The image is checked as libhdf5 checks it +/// (`H5C__decode_cache_image_header`, `H5C__reconstruct_cache_entry`): +/// signature and version, the image length it records, entry types, rings +/// and ages in range, entry addresses inside the file and not repeated, +/// flush-dependency parents that are earlier entries. +/// +/// libhdf5 does not verify the block's trailing checksum when it loads an +/// image, so neither does this. +pub fn apply_cache_image( + data: &[u8], + location: CacheImageLocation, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let bad = FormatError::InvalidCacheImage; + let start = usize::try_from(location.address).map_err(|_| bad("address out of range"))?; + let len = usize::try_from(location.length).map_err(|_| bad("length out of range"))?; + let block = start + .checked_add(len) + .and_then(|end| data.get(start..end)) + .ok_or(bad("image block extends past the end of the file"))?; + let eoa = data.len() as u64; + let mut c = Cursor::new(block, bad(RAN_OFF)); + + // Header: signature, version, flags, image data length, entry count. + if c.take(4)? != MDCI_SIGNATURE { + return Err(bad("bad metadata cache image header signature")); + } + if c.u8()? != 0 { + return Err(bad("bad metadata cache image version")); + } + if c.u8()? & MDCI_HAVE_RESIZE_STATUS != 0 { + return Err(bad("MDC resize status not yet supported")); + } + if c.uint(length_size)? != location.length { + return Err(bad("bad metadata cache image data length")); + } + let n_entries = c.uint(4)?; + if n_entries == 0 { + return Err(bad("bad metadata cache entry count")); + } + + let mut entries = Vec::new(); + let mut seen = BTreeSet::new(); + for _ in 0..n_entries { + let type_id = c.u8()?; + if type_id >= MDCI_NTYPES { + return Err(bad("type id is out of valid range")); + } + let flags = c.u8()?; + if c.u8()? >= MDCI_RING_NTYPES { + return Err(bad("ring is out of valid range")); + } + if c.u8()? > MDCI_AGE_MAX { + return Err(bad("entry age is out of policy range")); + } + let children = c.uint(2)?; + // libhdf5 checks the parent flag against the child count only in + // debug builds (release builds refuse any entry with children); the + // image format's own rule is checked here. + if (flags & MDCI_ENTRY_IS_FD_PARENT != 0) != (children > 0) { + return Err(bad("flush dependency parent flag and child count disagree")); + } + c.uint(2)?; // dirty dependency children: reset for a read-only open + let parents = c.uint(2)?; + if (flags & MDCI_ENTRY_IS_FD_CHILD != 0) != (parents > 0) { + return Err(bad("flush dependency child flag and parent count disagree")); + } + c.uint(4)?; // LRU rank + let address = c + .addr(offset_size)? + .filter(|&a| a < eoa) + .ok_or(bad("invalid entry address range"))?; + let size = c.uint(length_size)?; + if size == 0 { + return Err(bad("invalid entry size")); + } + for _ in 0..parents { + let parent = c + .addr(offset_size)? + .ok_or(bad("invalid flush dependency parent offset"))?; + if !seen.contains(&parent) { + return Err(bad("flush dependency parent not in the image")); + } + } + let len = usize::try_from(size).map_err(|_| bad(RAN_OFF))?; + let image_offset = c.pos; + c.take(len)?; + if !seen.insert(address) { + return Err(bad("duplicate addresses in cache")); + } + entries.push(ImageEntry { + address, + image_offset, + len, + }); + } + + let mut out = data.to_vec(); + for e in &entries { + // address < eoa <= usize::MAX, and the entry's bytes came from the + // block, so neither conversion nor the sum can fail. + let at = e.address as usize; + let end = at + e.len; + if end > out.len() { + out.resize(end, 0); + } + out[at..end].copy_from_slice(&block[e.image_offset..e.image_offset + e.len]); + } + Ok(out) +} + +/// What a reader must do before reading a file's metadata, in one call: +/// check the superblock extension ([`read_superblock_extension`]) and, +/// when the file has a metadata cache image, return the file's bytes with +/// the image applied ([`apply_cache_image`]). `Ok(None)` means read `data` +/// as it is. +pub fn metadata_view(data: &[u8], sb: &Superblock) -> Result>, FormatError> { + match read_superblock_extension(data, sb)? { + Some(SuperblockExtension { + cache_image: Some(location), + .. + }) => apply_cache_image(data, location, sb.offset_size, sb.length_size).map(Some), + _ => Ok(None), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sb_v2(ext: u64) -> Superblock { + Superblock { + version: 2, + offset_size: 8, + length_size: 8, + base_address: 0, + eof_address: 0, + root_group_address: 0, + group_leaf_node_k: None, + group_internal_node_k: None, + indexed_storage_internal_node_k: None, + free_space_address: None, + driver_info_address: None, + consistency_flags: 0, + superblock_extension_address: Some(ext), + checksum: None, + page_size: None, + } + } + + /// A file whose superblock extension (a version 1 object header at 48) + /// holds the given messages, padded to `len` bytes. + fn file_with_ext(messages: &[(u16, &[u8])], len: usize) -> Vec { + let mut body = Vec::new(); + for (t, d) in messages { + let padded = d.len().div_ceil(8) * 8; + body.extend_from_slice(&t.to_le_bytes()); + body.extend_from_slice(&(padded as u16).to_le_bytes()); + body.extend_from_slice(&[0x14, 0, 0, 0]); + body.extend_from_slice(d); + body.resize(body.len() + padded - d.len(), 0); + } + let mut f = vec![0u8; 48]; + f.push(1); + f.push(0); + f.extend_from_slice(&(messages.len() as u16).to_le_bytes()); + f.extend_from_slice(&1u32.to_le_bytes()); + f.extend_from_slice(&(body.len() as u32).to_le_bytes()); + f.extend_from_slice(&[0; 4]); + f.extend_from_slice(&body); + f.resize(len, 0); + f + } + + fn fsinfo_v1(page_size: u64, persist: bool, n_addrs: usize) -> Vec { + let mut m = vec![1, 1, u8::from(persist)]; + m.extend_from_slice(&1u64.to_le_bytes()); + m.extend_from_slice(&page_size.to_le_bytes()); + m.extend_from_slice(&0u16.to_le_bytes()); + m.extend_from_slice(&u64::MAX.to_le_bytes()); + for _ in 0..n_addrs { + m.extend_from_slice(&u64::MAX.to_le_bytes()); + } + m + } + + fn mdci(address: u64, length: u64) -> Vec { + let mut m = vec![0]; + m.extend_from_slice(&address.to_le_bytes()); + m.extend_from_slice(&length.to_le_bytes()); + m + } + + #[test] + fn no_extension() { + assert_eq!( + read_superblock_extension(&[0; 64], &sb_v2(u64::MAX)).unwrap(), + None + ); + } + + #[test] + fn file_space_info_as_libhdf5_decodes_it() { + // What FileWriter::with_page_size writes. + let f = file_with_ext(&[(MSG_FSINFO, &fsinfo_v1(4096, false, 0))], 256); + let ext = read_superblock_extension(&f, &sb_v2(48)).unwrap().unwrap(); + assert_eq!(ext.file_space_info.unwrap().page_size, 4096); + let f = file_with_ext(&[(MSG_FSINFO, &fsinfo_v1(4096, true, 12))], 512); + assert!(read_superblock_extension(&f, &sb_v2(48)).is_ok()); + + let refused = |m: Vec| { + let f = file_with_ext(&[(MSG_FSINFO, &m)], 512); + read_superblock_extension(&f, &sb_v2(48)).unwrap_err() + }; + // Persisting, but too short for the manager addresses. + let mut short = fsinfo_v1(4096, true, 12); + short.truncate(short.len() - 8); + assert_eq!(refused(short), ext_err(RAN_OFF)); + assert!(matches!( + refused(fsinfo_v1(256, false, 0)), + FormatError::InvalidSuperblockExtension(_) + )); + assert!(matches!( + refused(fsinfo_v1(0, false, 0)), + FormatError::InvalidSuperblockExtension(_) + )); + let mut v2 = fsinfo_v1(4096, false, 0); + v2[0] = 2; + assert!(matches!( + refused(v2), + FormatError::InvalidSuperblockExtension(_) + )); + // cve-2020-10810: version 0, strategy ALL_PERSIST, and a message of + // 32 bytes that cannot hold the six addresses that follow. + let mut v0 = vec![0u8, 1]; + v0.extend_from_slice(&[0, 1, 0, 0, 0, 0, 0, 0]); + v0.resize(32, 0xff); + assert_eq!(refused(v0), ext_err(RAN_OFF)); + // A version 0 message without persistence is fine. + let mut v0 = vec![0u8, 2]; + v0.extend_from_slice(&[0; 8]); + let f = file_with_ext(&[(MSG_FSINFO, &v0)], 256); + assert!(read_superblock_extension(&f, &sb_v2(48)).is_ok()); + } + + #[test] + fn cache_image_location_must_be_inside_the_file() { + let f = file_with_ext(&[(MSG_MDCI, &mdci(128, 64))], 192); + let ext = read_superblock_extension(&f, &sb_v2(48)).unwrap().unwrap(); + assert_eq!( + ext.cache_image, + Some(CacheImageLocation { + address: 128, + length: 64 + }) + ); + // cve-2020-10812: 256 MiB at 0x10100 in a 2565-byte file. + let f = file_with_ext(&[(MSG_MDCI, &mdci(0x10100, 0x1000_0000))], 2565); + assert!(matches!( + read_superblock_extension(&f, &sb_v2(48)), + Err(FormatError::InvalidSuperblockExtension(_)) + )); + let f = file_with_ext(&[(MSG_MDCI, &mdci(u64::MAX, 8))], 256); + assert!(read_superblock_extension(&f, &sb_v2(48)).is_err()); + } + + /// A cache image block with `entries` of (address, bytes). + fn image(entries: &[(u64, &[u8])]) -> Vec { + let mut b = Vec::new(); + b.extend_from_slice(MDCI_SIGNATURE); + b.push(0); + b.push(0); + b.extend_from_slice(&0u64.to_le_bytes()); // length, patched below + b.extend_from_slice(&(entries.len() as u32).to_le_bytes()); + for (addr, bytes) in entries { + b.extend_from_slice(&[5, 0x02, 1, 0]); // type, flags (in LRU), ring, age + b.extend_from_slice(&[0; 6]); // children, dirty children, parents + b.extend_from_slice(&0i32.to_le_bytes()); + b.extend_from_slice(&addr.to_le_bytes()); + b.extend_from_slice(&(bytes.len() as u64).to_le_bytes()); + b.extend_from_slice(bytes); + } + b.extend_from_slice(&[0; 4]); // checksum (not verified, as in libhdf5) + let n = b.len() as u64; + b[6..14].copy_from_slice(&n.to_le_bytes()); + b + } + + #[test] + fn cache_image_entries_replace_the_file_bytes() { + let img = image(&[(16, b"HEADER"), (40, b"NODE")]); + let mut f = vec![0u8; 64]; + let at = f.len() as u64; + f.extend_from_slice(&img); + let loc = CacheImageLocation { + address: at, + length: img.len() as u64, + }; + let out = apply_cache_image(&f, loc, 8, 8).unwrap(); + assert_eq!(out.len(), f.len()); + assert_eq!(&out[16..22], b"HEADER"); + assert_eq!(&out[40..44], b"NODE"); + assert_eq!(&out[..16], &f[..16]); + + let bad = |img: Vec| { + let mut f = vec![0u8; 64]; + f.extend_from_slice(&img); + let loc = CacheImageLocation { + address: 64, + length: img.len() as u64, + }; + apply_cache_image(&f, loc, 8, 8).unwrap_err() + }; + let mut sig = image(&[(16, b"x")]); + sig[0] = b'X'; + assert!(matches!(bad(sig), FormatError::InvalidCacheImage(_))); + assert!(matches!( + bad(image(&[(16, b"a"), (16, b"b")])), + FormatError::InvalidCacheImage("duplicate addresses in cache") + )); + assert!(matches!( + bad(image(&[(1 << 20, b"far")])), + FormatError::InvalidCacheImage("invalid entry address range") + )); + let mut len = image(&[(16, b"x")]); + len[6] ^= 1; + assert!(matches!(bad(len), FormatError::InvalidCacheImage(_))); + let mut cut = image(&[(16, b"abcdef")]); + let n = cut.len() as u64 - 8; + cut.truncate(cut.len() - 8); + cut[6..14].copy_from_slice(&n.to_le_bytes()); + assert!(matches!(bad(cut), FormatError::InvalidCacheImage(_))); + } +} diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index 3232ee6..8746f5f 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -46,6 +46,9 @@ pub struct LazyFile { /// End of the HDF5 data (`Superblock::data_end`, absolute). end: usize, superblock: Superblock, + /// The metadata as libhdf5 reads it when the file holds a metadata + /// cache image (see `superblock_ext::metadata_view`); `None` otherwise. + overlay: Option>, root_header: ObjectHeader, /// Cache of parsed object headers, keyed by address. header_cache: RefCell>, @@ -82,7 +85,13 @@ impl LazyFile { let superblock = Superblock::parse(data, 0)?; // Refuse a truncated file; read nothing past the recorded end of file. let end = base + superblock.data_end(base as u64, whole_len)? as usize; - let data = &reader.as_bytes()[base..end]; + // Decode the superblock extension as libhdf5 does at open, and load + // a metadata cache image over the file's metadata. + let overlay = clawhdf5_format::superblock_ext::metadata_view( + &reader.as_bytes()[base..end], + &superblock, + )?; + let data = overlay.as_deref().unwrap_or(&reader.as_bytes()[base..end]); let root_header = ObjectHeader::parse( data, superblock.root_group_address as usize, @@ -94,6 +103,7 @@ impl LazyFile { base, end, superblock, + overlay, root_header, header_cache: RefCell::new(HashMap::new()), }) @@ -111,7 +121,10 @@ impl LazyFile { } fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..self.end] + match &self.overlay { + Some(v) => v, + None => &self.reader.as_bytes()[self.base..self.end], + } } /// Returns a reference to the parsed superblock. diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 2a8400b..05d581a 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -38,6 +38,9 @@ pub struct MmapFile { /// End of the HDF5 data (`Superblock::data_end`, absolute). end: usize, superblock: Superblock, + /// The metadata as libhdf5 reads it when the file holds a metadata + /// cache image (see `superblock_ext::metadata_view`); `None` otherwise. + overlay: Option>, } impl MmapFile { @@ -50,18 +53,28 @@ impl MmapFile { let superblock = Superblock::parse(data, 0)?; // Refuse a truncated file; read nothing past the recorded end of file. let end = base + superblock.data_end(base as u64, whole_len)? as usize; + // Decode the superblock extension as libhdf5 does at open, and load + // a metadata cache image over the file's metadata. + let overlay = clawhdf5_format::superblock_ext::metadata_view( + &reader.as_bytes()[base..end], + &superblock, + )?; Ok(Self { reader, base, end, superblock, + overlay, }) } /// The file's bytes from the superblock on — the space HDF5 addresses /// index into. fn hdf5_bytes(&self) -> &[u8] { - &self.reader.as_bytes()[self.base..self.end] + match &self.overlay { + Some(v) => v, + None => &self.reader.as_bytes()[self.base..self.end], + } } /// Size of the user block before the superblock (0 for most files). diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 40ac08b..c892eea 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -20,6 +20,7 @@ use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_format::signature; use clawhdf5_format::superblock::Superblock; +use clawhdf5_format::superblock_ext; use crate::error::Error; use crate::types::{AttrValue, DType, classify_datatype, read_attrs}; @@ -55,6 +56,11 @@ struct FileData { base: usize, /// End of the HDF5 data in the file (`Superblock::data_end`, absolute). end: usize, + /// The file's metadata as libhdf5 reads it when the file holds a + /// metadata cache image: the bytes from the superblock to the end of + /// file with the image's entries written in + /// ([`superblock_ext::metadata_view`]). `None` for every other file. + overlay: Option>, } impl FileData { @@ -68,11 +74,26 @@ impl FileData { let end = superblock.data_end(base as u64, whole.len() as u64)?; // data_end is at most the file length (less the user block). let end = base + end as usize; - Ok((Self { backing, base, end }, superblock)) + // libhdf5 decodes the superblock extension at open (a message it + // cannot decode fails the open) and loads a metadata cache image + // over the file's own metadata. + let overlay = superblock_ext::metadata_view(&whole[base..end], &superblock)?; + Ok(( + Self { + backing, + base, + end, + overlay, + }, + superblock, + )) } fn as_bytes(&self) -> &[u8] { - &self.backing.whole_file()[self.base..self.end] + match &self.overlay { + Some(v) => v, + None => &self.backing.whole_file()[self.base..self.end], + } } fn len(&self) -> usize { diff --git a/crates/clawhdf5/tests/fixtures/h5clear_mdc_image.h5 b/crates/clawhdf5/tests/fixtures/h5clear_mdc_image.h5 new file mode 100644 index 0000000000000000000000000000000000000000..6ed8b702cfbd9ae5f61ea2698fb65d404cddf7ef GIT binary patch literal 23467 zcmeI)f1H%_{{Qi5`(dqqtZZ3HrX;IhR;?^nrfgZMtRySRsFg*s5@(UlU}+_6>9DdG z35yOZ$x572o$MhjhpZHXBq57*Rwwm+Jeu+Oer})J_Yb$*_pi_Ax)`>??pPyJ~Cp%tmof1ht{A2aU9r)u8{BZ~V zcin+A3-ZUL1Q)dbx#&OZoOxiNJP;h0xEFUM2hYHmU5`%q-*tQb_}Bi=z5^}z=cS+} zTESO(8|()^J?sw`_p#qT+tbh%?U9a-aPc_qc|God@cakCb99F1=>pHy4MFsP=j;XBx+k}fV%;14;Af_R7=#m$ zg}$&o2f4^Y2>B?$Nc`T#>+?Q&jR(W)JOo~=_s#1)44&6(^4$I5`3Jyj@Y=kN6XCU- zf}t1&ujdSm##tDPb5WRlT*P_;!YD=wrlJ%x@Ou~U-{Av&v%FeYOPE``^2C8pyVlwl?!C`Se6q7n;Gg=+lXC5s)8hWEM; z`ohn>2 zyxynr44y+h)}aCKqY)bs$7XEBxA?tF9y^{4Uo*b0Ll^>IJEtNazRtbR*JCEU*Ad)| za@>Xrc+b73FJc8=K@6+07XN_P{tlY31I^ftKnun%q@pd-k%8a43}?sFP=M1h0=_Rt zVibJMpNYA+3ze9U1@PWiVIivV5Z=QF@cw>`Pw*MM$6sL^zC$M0*cCzaL>BrY8#%~D z9)9mq$c~O9V{kUc;v9^F<4O_ELlnM77UOZ$;3+JDua#OXh4=XOh6dL_`S<`c03;w;5ai87b1*{FbR|4>t#9W;A>_jUPTP6um-+% z+H>v$;cMbxbirZpz8`^JI2t9Gic-u#8D=7aa#Ub0Tv{Uy9gu-T&<#f*3yw=JZO|4S zkqP(diQdS@Ah_&@_UMGJ@Hp<{c$N#_UoNRghsO%Sb9o&1384Tk`@?e{3eV^DcrK6Q zKE6%{z;P!JLogh^?hE1T#=nF2-TtBQdA=8XUD+4L6imf*_9o2%`k{mmz`*RH6!=V=_GcHSqbfVZYg4tc)-ia@b$AaR_Y-V}$F}Vw#PJoj<7arjmYgpQ zzAu{CZbpE|zNZ~O9h-v4g5!~6k?$klKeqdR8Gw`Edw&$h;X-&1-h$`<5F7Csw%{9h zKCi>|PIx_zDW2PN_&)VL>Un%`dM$@1ANOOO1J64Yr^E5)T!c`N+%94rMhSdBmmz`* z*mf1J$1U(Z{TDohCs2!*u#4;3i&i||A70x*@Om6Sdf+JZh4&&CgW>%c0k6w@;utsy z6LB#dW3GnhzX@K`-MAMIq7qezq6S`19b%|Q0~)akZ{mGy!WMjwX0%A*?~8-r^&N{r z@Vo1ZeVK+C@Y=jzx54`}A6}F9Xfd9~Qh1-duGjH4KEOtNfjG9J30_lx zYe_{qG7&@;y5dOm$MFc^bd16ID8?1={!ND0c?G89PjHO(K63wW@8vr z;nEIWa1?Sd1ZJ%HSpsv1d8|n&g}GLYX(+?Zm#tI>c>FpK^Lug7zF9QRoRGa`=d z*o`z^FZHZ#--s<}!Y;I;uRSu+1NNJ1KjK%U(&qTw4dzo0PJz$&*!E{22*<=+ASc-)Tg zoW0=l9gp%c8nzt^&vzOen=XLIy&AJ{JBm;YuVW@EumGNK2^>?`U=HrXVmt?P;7vG& zeTg5j7c21wY~O+%NMU?(JnM$OI1!$A5~jj`X9S+Zb3cR{EKP1d%=#%fZpPqw*W)vM zi+`a38?hChGr&38!ZEZbvf&tN+ac%$$JG!trtA7{ezUkuMX7GX?98Ol)!&s~FM@LC>7Zoj~K z72d%{e2txG*^2iI>(9}IJ!nf?7i3`&@-YTM`plXf42IV+1`{v^rMN!1Hg9gjJa|n{ zU@2ZfJ^qOp)}axb;rVwXm2+iyKJ-O84nuDo53lDeOvL3V!>tHo8fIb+s_;0LVHFzi z3A|U{n<^~A5-f+;^9H;hpCoyoc3>~kcy1?nU47ua8G>ARO@)|%63jpZbK!L?#(X@C zC3p#I@lV9@9d=>A*1TSmk5hTv89gxoA&kZZTn3j8Fw6TP4`*OJtP{<(*>Fim5ZMT! z5Mh)ef=aj?h#nY-e4K+Ru&%9u^{7in^u!<(z;l}Q=Cj9I1eb%*3nySCth1-%R#f3> znB9-T%v}vL@iR2Rx}-g0nRQ5%$F+!|0dcrbfWCADkqtisnD1t{xoqZo9QWyt0XP+& zZ!OGsv)kXQH=9Hg<%@sfQ zhERwwN)bUNqNqg-4HygSD06Z)s^I5mbM8%;W8cBNydGxdL+}{pmN~cu=1&@9jX8Wb z%-I)Uj`}%vJFKHJa1?y5pL63d*US>@g|6rWGsJwdzX<^zrz41LgiweuO5yW6u(sd4 z845qQn<>}eHkcb8zdwAwS!BQCzIn3{W{sKR`A5Oyn^QjD&(HS%4e!CcF+VPW$G8>d zhINXW@fsTNDI%yu6t#%KzBrl?;Ju#?^X4U(T_3|-v0i9T-{Hu{5cu5BVaBv!+wt6S zGY_K?hI!%me*kMU?^rm_`?>ux+=xm%3bVrVm=lM>W0?z%zwYnYdkuo{8bT<9=P5-5 z9xIAkIA%}A4BU!);rLvKe_#W?f#alo=E({;Ry!^>A*EGfjPAwSvD4>m#P`WQ$4T?! zB#g#I@ObyYG50ydU>t9g~-%9vk48JsS)0SH$4>yam6YEo1KCFdL7C$2|?>a4~%TE%4Zn zAdFIYzDh(_Iw@dmsnHQ3Uf~Chme+F$}g} zj#-$8#dr~KU=zHSzu|3cg2(r~Y4ja}EF6yjJZBKu2%!*Rlp=yk*me=7;}$HyV_1&Y z(TJ}R#Zs)ndThlmv~9!R64@AvCfZuE?u?_6hcht=*PsFqzMCG4=x$Vf>~aOVwmIBlu?*jE}hUDr(i71d$ZU1+!{C!>(Uv0F%(6x zCN78dsad)fE(fDOhG7D(#vD9?I=lmyOk~5%55sG#gy)Q*5%t)JCbVMg&xF|+f|+Js znMLOOS1@<2M|;8iEQGmdrkPhB>qn&WxEpd{#)e@wntNv27WT0|&fu|i?NFG*r7%Cu zMsv^W@>oIS!t;4e5ty;Hs7EEt>^iJN96ON8SZr2hBOmVb608TmKr_tC9&o;DBrbx- zai7hwR!pa@7xG})46_k3B{<%(gJfPzf_A2x|}fCZH5%N)`MJAA_IW?Y{tK>}*uS zyjhEnVWzZZob&m&!G3F#dYDCi-uJV95Prt@_|L-UZ-V{iR1chhLREDfX9`}U%j6@h_NCm24&X{ZYI1gr*`QkCmr&ah6KG)1E!(A}fR-gf9 z*KTxVOq<2pJY0g+*nl5konfvG!YI_zW){WKjC9(}f)LD$ec*8%ug$DK!<={$K5sC_!oCRZ#S*N7`LP`y^9Ia^&x_$h*w>n|)p2_u zqHNcp0b3EEEfbFM1qj2jyNtfMaQ|hf$3`@v75zS^FU*mD!ejl4joBhDJ0YmG&TVQHT8bqf5ke9Cw{?Zb%|{WY!sjnQ4OU_j{shnW2$thbc-)_n z#<32E`@RRSV-GTD>kZF04pVV6I-?)*aXzNwc0}&@p}jKNe` z`$n+>@55TNJ)F1Bhx5(my!ElUYh7#2YCW3C+N>{x$Etu?8-um4*_Xk7Jy<*cJptF@ zURY;0!1~zQ*Sgm1HkZv@^VD3t3UgpxXeNGuRG8i7vYBh1y3b-*e>UPrbcOkDcALv) zuE%j7zgIFhJ21{?v-UGU3C!*&JeS9DpLF)=i+mpYIl<2W=6fBy9?#`*+~;&y5AMf0 z2nbz77eX+PXW%Z>U==oEC#)y^-Y5*`?X3CC%XRn)DU9FFznQDW zh``LOgL!M-wWU7`X4Mq7Z$T8VzGW_+%S)ppdMyX zfHuc}b7(9|;TUg*eT_CeHm5?k0A|sBSO)WEJ^OqQbEG>?#Mv-IZpA}*5uRfZ3UL|A zVP750i*L~;HIXwn^0*4m<86G6R`i)KC*mBKOKT9vZe-GygF=`wX2xP{r|$sPX5KJN z!khOw9i&$9?C;jx;KMjn}4ci?fnhK=|a zI?~rKnOT!r$C7=`tTWg)&qksI<%nV>(&6();1byP2wuge2)Gn1TvCg>~4DboOyx+Z?xk zw&pdT%~Ufyjy*8foMRot<8v?re$ToT&I>oe`CR8}M3*22y&r_<-WusP~{?pWHa>#N}B2D9Gl@>oMr4Cj2!Z);ZT(WS7CG#kx5GtIm* zi>!UEYpq$WN3Au@PqWe7GtPFkin1CCw z5HG>wxKDt`L4*)S1X08ghv)J*K^_mmBv@aXn}3J3qu)n)J)SEB_bJEYuts#Q(s@dA z&AfgaW@#GRSuo42*PN4!!WycP{(r%G@HmWx^LEyA&!GX|BY+@62qS_hVu&Na7$1b^ zIUSe5|I3v1qIqdO*n$3RSf3TLJsoD|ax}uZzOM8+#}|fkd*-0+=G-8dV`iW^E@^MmH#;k=?b`T@+zgK0MhogXyk z9)LN#7Upak+vccsnK?Nh=HPmmf4yK0Wvvu}`DQ(|0nM=Hv41jdhILd7pTb({F#7Xg zJ>>JFus@CfZDvmhVfzt<$KM2>Z$8?85v;Ku#Tr;gdHyb}J^n=ad~?hGEl6eT?hSKq z49u^)umom`HCG$@&5fbBp6&Z#P4*#n!o11B*)UV)Ax3)~)@wn85Jm)1#1MzaJ_^=y zmtqdgsde}cX3;S)8>gWXOYuH-B9p!oF#)p>rp@fC#tN8E?w3KI8B~aAsHM$Z+m3c@ zn_Xw%GMG;_cpd(j%ghTQj0mELAr61U6@}n2Ck3uQltQI2Ds{6QcMVKEbc>d0|9gUkq{h@A`MOcj;^mS&Pi}9F=hY-W(XvL4wdijGimLo76mtrog%gu6s zw`=|Be6&kX6ksYUVdh(JTSo`k$6D6qNQ}UhSb&%C30l&hg;B}-Db*^=%-?N&Lem`p7o4vJoAJ)%i zeF$c%_3IK$r`_LUt%Sd$YD1gf`xfFF_+9MZFq6JTsE2iZ25o+B2*Z5#Snv6mKJ(sl z{u|cTet%qw`(VB7_qEo<&Nn-k>^!mgZCz{4YCUSLX&q_KTQ{4#c?iP{t${VG^{BPx zX7+L3x<8z6E`#;4wXbz;TlTRYZDv2`w)5efbS12zHzI|8>so78KUX;aZO%Jy?fI;; zt)Z=t^I=_UcALv#n5Sl?b*#DAfm|5?=ReJNvpWJa*F1Hfboz2o1ZzM)1DNkon9F9a z$8n$YFbj{u&k248h{5bOmpzxqai7Jo&h)c{pA+IR-_35X$8&ic_xSv()F!fc2hP7Dp@kt=C4vdh$Y8FFpzD!JlBgcQPhnHmo5dqrFNONo| z%qeRpbM#%9lZUb$!o@J>o`pH~3(VQ}tj*E!h`_qc9Q+aH++cW)GhohIpT+PM%*o?m z4&H+Kw7mfH@89S~+Ynfr&4$_X7VO`RAdmA=0<)(UW>_=2nggs)h5dJ8DL%w*bfm(>15j= zWb^o3%)(<>ho8`ez9A^V-55>VRak%*u>rp#NZ+ZLg4)4J?9eACy zWT6n&=26tcy4)-e!3=ac8e?z+7UM1K#NqT8;Hu<)y=fbZS*U@Z4b1#r^cTXpYnMJ4 zhnuhjA0mamekj5%aOsO8l%p0KkxE|bT=WDHs_a}J9b{!IbY{dK8Gq21dYhUZyAlufX)|%$0*=X*WY37v~Z|xg`HLLZg z8Ebx;jpm-2md3XAaTwOM)~x2R8Ebx;jpm-$G!_w9`!>R?Hiykv^V4h$p#H9_H5_EY@G|gi;E+T?C@M3$9+yfG493+ ze2H}SISF2m=khr2Qwj5TD>CR0!FkjMwwn>;aRJP`Dwv06mGhFhNa1lGjKz&`-t!$a zqdWa0a5creA`Da=eWH5=w#IiA53+UH>>=!#1HS+ve^J zL}6w&qbJ!BW?cd6J+m%ATNbR>%3wWajb**~cUTV|%ytNu!g}syY(_gW!DFQG*m}`9 z&zIiec#HsTePO*=4(mZ{F>^4JwK-P; zbF2>Llo@Ca6~Uam5a!@>Fy~s*ZjPM;bLt70qmRR!+y!&cj56n}@ywu|FlWC*CXY{r zIe9aZE+WFE}H1&GjQ#xXb61%2-yD+>hUAG(l;EXSm>Iz2jKI6NcNvb`*b{rdi(^>Z@zl`eE9ro*uMio zGH@i)d7O(dDo~3?_+#-b6rv25(RL43;!Cur?RbQ7JC@>Otf9}0&E)Y=OvSy3VH+~i z$sTm$@#&a>hw%n}Mv%TUa4jBz=U$8k>_!%CV-dj;G@_M1L}%#-=h|n(n)@@fCwr{R z&2qmtcIl4^n1ecOK?Z$8QHqC>`yE63MBIs$_y%3*I~`?sBDr5S?O{|Rh9(5*GxH;; zP40Ir?UUf|?$*G013w#_iwgL=Fz42t=eE|i?lpV;p4c4scfHP4`#aus?6V75w2ebK zoY#)S`QtoTpRQ-0*0kkdGUnqoG{N5?kH(FNv!59qLMfu~oB{f>5k@7PFF2HTKNsAH zXJ8$kPTvrixsSpc+dABNV}Ji-hF=WpNxx?`*Zh9etT&&{)E=}IViuNQ15(*%5SGyI z@3GQo^LJDw@O$62@cY`H?Bn;WG4^RfkT#DMftehKwQ>lhaDJdQZT@D<-(&fCz`A=E z{N8s0?u2vF&NoNlJh5}V*0t8G)}z*%){(`u&qoZ-C3mIG+P4hWtk$E}n$B%IZyks8 z&CVrTA6LV=)|%CN)blkUz_!^Oh8Y?|6N2nh2tNxr{~e^wd28pSt+UNp>tkzQ>sqth zTsCvfQ?t_V3p^7IpT=Ud@^5Jh>qVTsZeg-h#&2Dqq%=I|#^9UNS z7ZLhu5l1@P=CbGVIPMc5hs;htOZYj#&j9AT+3odsE|23r7s0t)zlXGbJPXc|I?rmJ z`MsWVqZeW~`#2w5gxg>Z`#I9-v;Lflh3HS)1(=JMu?>gNUVuNr@Ap1{`CEw?nh>PT z%riG@5l1@tV!gT=J8&dzW~+5-9lk~v_BjLArZ2&HPwPVS(7JIUUWYYbcXGo_-wEeM z&Ep83fmxbHe=eq=725+b3G)!c_vl97C^)bAG(1l!qA)W9w3)f)T_s{@g1P!vIPcnl z?GUW{%+!CvdC}uw*4_ZK@B>)S9Ruq%>p<(VFJQg63Ld)~+tz#6qXz3?y*2>Wll!x6 zy?6<%2mcQ1y(8$ip1TR=Wg#M{MI2_C^`Kd2z1INi(feSY{sQaC^I^UC3e2j*$rYb_ zGR(RAV2=F&bLtG3Lyy6nybI>wHkfn6VU8_=IkgA1^nD6*@?@BU55S!J1?JefFlV!2 zj#k2)Y=Svh2y?F1$F$u9bM_0Eqr+fMJ^^!Ze{%3W`u3tP+vnjnJdZfq(SIT?f&JrA zf#ukYj`@=rZIQOR(O0E2R%a|@zp-?EqG)8t0iWZUo|1^J9}X~oHH=Z;SYgPl;{x48bTX8;M{GJHt>Q2tiG?ZUX>QlDJ=o_8tYJ?IkjJ;Sm!xvu5)>QcsQ z%f;89ODcZ@mxkoJihJl1=HKg*#lO*|DY>pYD6t*MNURI|(LPIh=ft`(xsLkddzO;S z#5&s_4YD*R*Rd{%?FxVV$5NQQL7Dz|k0qX5*K|v4m-^!amfYaJ6@yM+AuEb#rnZ8<^Ox$VsdVk4vmG2PM|=e7^4*9}W-M}{ZX1*av}=>>^(V{#qkUExwPBC*asBe8Byu45w;+ZCe{>%!5A zb>^9gbv(JQDNJmao|RbVj!CTd{YSERSbuh6yV40QmZEbK>o-#`tZMxq@23Rqj|8x1kJ39T}JLlk&d@1j%2mB}ezyC9@WhwmA8PxyF plE!V}(_sgH`O_-@>|tYn`#S`ZuOy%J|FsPM_1UfiH`RUhe*p5$_ErD@ literal 0 HcmV?d00001 diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 0bfd30e..74dc419 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -42,8 +42,9 @@ macro_rules! skip_if_no_python { /// Runs `body` (Python, with `h5py`, `numpy as np`, `struct` imported and /// `d` the output directory) and then, for every `NAME.h5` it wrote, -/// prints `NAME ok` when h5py opens and reads dataset `d` and `NAME ERROR` -/// otherwise. Returns those lines, sorted. +/// prints `NAME ok` when h5py opens and reads dataset `d` (or the dataset +/// the body names in `DSET`) and `NAME ERROR` otherwise. Returns those +/// lines, sorted. fn h5py_verdicts(dir: &Path, body: &str) -> Vec { let script = format!( r#" @@ -54,7 +55,7 @@ for path in sorted(glob.glob(os.path.join(d, "*.h5"))): name = os.path.basename(path)[:-3] try: with h5py.File(path, "r") as f: - f["d"][()] + f[globals().get("DSET", "d")][()] print(name, "ok") except Exception: print(name, "ERROR") @@ -78,14 +79,14 @@ for path in sorted(glob.glob(os.path.join(d, "*.h5"))): lines } -/// Whether clawhdf5 opens and reads dataset `d` of `path` (as raw bytes of -/// whatever type it has). -fn clawhdf5_reads(path: &Path) -> Result<(), String> { +/// Whether clawhdf5 opens and reads dataset `dset` of `path` (as raw bytes +/// of whatever type it has). +fn clawhdf5_reads(path: &Path, dset: &str) -> Result<(), String> { let file = File::open(path).map_err(|e| format!("open: {e}"))?; - let ds = file.dataset("d").map_err(|e| format!("dataset: {e}"))?; + let ds = file.dataset(dset).map_err(|e| format!("dataset: {e}"))?; ds.dtype().map_err(|e| format!("dtype: {e}"))?; ds.shape().map_err(|e| format!("shape: {e}"))?; - file.read_multi(&["d"]) + file.read_multi(&[dset]) .map(|_| ()) .map_err(|e| format!("read: {e}")) } @@ -93,10 +94,15 @@ fn clawhdf5_reads(path: &Path) -> Result<(), String> { /// h5py's verdict for each file must be `expected`, and clawhdf5 must read /// exactly the files h5py reads. fn assert_agrees_with_h5py(dir: &Path, verdicts: &[String], expected: &[&str]) { + assert_agrees_with_h5py_on(dir, verdicts, expected, "d"); +} + +/// [`assert_agrees_with_h5py`] reading dataset `dset`. +fn assert_agrees_with_h5py_on(dir: &Path, verdicts: &[String], expected: &[&str], dset: &str) { assert_eq!(verdicts, expected, "h5py's view changed"); for line in verdicts { let (name, verdict) = line.split_once(' ').unwrap(); - let ours = clawhdf5_reads(&dir.join(format!("{name}.h5"))); + let ours = clawhdf5_reads(&dir.join(format!("{name}.h5")), dset); match verdict { "ok" => assert!(ours.is_ok(), "{name}: h5py reads it, we fail: {ours:?}"), _ => assert!(ours.is_err(), "{name}: h5py refuses it, we read it"), @@ -244,7 +250,7 @@ for libver in ("earliest", "latest"): ], ); for name in ["earliest_size2", "latest_size8"] { - let err = clawhdf5_reads(&dir.path().join(format!("{name}.h5"))).unwrap_err(); + let err = clawhdf5_reads(&dir.path().join(format!("{name}.h5")), "d").unwrap_err(); assert!( err.contains("stored datatype size in chunk layout"), "{name}: {err}" @@ -557,3 +563,68 @@ for name, n in (("overflow", (1 << 62) + 2), ("pasteof", 1000)): assert!(mm.dataset("d").is_err(), "{name}: MmapFile"); } } + +/// libhdf5 decodes the superblock extension's messages when it opens a file +/// and refuses the file when one does not decode: cve-2020-10810 (a File +/// Space Info message too short for what it announces), cve-2020-10812 (a +/// metadata cache image past the end of the file). We did not look at those +/// messages and opened such files. +#[test] +fn superblock_extension_messages_libhdf5_refuses_are_refused() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let mdc = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/h5clear_mdc_image.h5"); + let body = format!( + r#" +{FIX_OHDR_PY} +def ext_addr(buf): + assert buf[8] in (2, 3) + return int.from_bytes(buf[20:28], "little") +def save(name, buf): + open(os.path.join(d, name + ".h5"), "wb").write(buf) + +# A paged file: its superblock extension holds a File Space Info message +# (version 1, strategy page, not persisting, threshold 1, page size 4096). +DSET = "DSET" +good = os.path.join(d, "paged_good.h5") +with h5py.File(good, "w", libver="latest", fs_strategy="page", fs_page_size=4096) as f: + f.create_dataset("DSET", data=np.arange(10, dtype=" ext +# Page size 256 (under libhdf5's minimum of 512). +bad = bytearray(data); bad[at + 8:at + 16] = struct.pack(" ext +length = at + 5 + 8 +bad = bytearray(data); bad[length:length + 8] = struct.pack(" PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/h5clear_mdc_image.h5") +} + +fn expected() -> Vec { + (0..50).flat_map(|i| (0..100).map(move |j| i * j)).collect() +} + +#[test] +fn file_reads_through_the_cache_image() { + let file = File::open(fixture()).unwrap(); + assert_eq!(file.root().datasets().unwrap(), ["DSET"]); + let ds = file.dataset("DSET").unwrap(); + assert_eq!(ds.shape().unwrap(), [50, 100]); + assert_eq!(ds.read_i32().unwrap(), expected()); + + let file = File::from_bytes(std::fs::read(fixture()).unwrap()).unwrap(); + assert_eq!( + file.dataset("DSET").unwrap().read_i32().unwrap(), + expected() + ); +} + +#[test] +fn mmap_and_lazy_files_read_through_the_cache_image() { + let mm = MmapFile::open(fixture()).unwrap(); + assert_eq!(mm.dataset("DSET").unwrap().read_i32().unwrap(), expected()); + let lazy = LazyFile::from_bytes(std::fs::read(fixture()).unwrap()).unwrap(); + assert_eq!( + lazy.dataset("DSET").unwrap().read_i32().unwrap(), + expected() + ); +} From d110b1d9451945162356267e6f49b0b607ea8e63 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:31:52 -0500 Subject: [PATCH 08/20] fix(format): scale-offset and shuffle decode chunks as libhdf5 does Scale-offset (filter 6) now follows H5Z__filter_scaleoffset: - the packed codes start at byte 21 whatever size the chunk records for minval (libhdf5 reads min(8, size) bytes of minval and always starts the codes at buf_offset 21). We started them after minval plus 8 bytes, so a chunk recording a size of 0 (cve-2025-44905 /Scale_offset_short_data_be) decoded differently from h5py; - with a fill value defined, a code equal to the all-ones code of minbits bits is the fill value, including minbits 0 (code 0): a chunk of nothing but fill values read as minval; - minbits of the full width stores the elements as they are (no minval added), and an integer scale factor of the full width means the chunk was left untouched; minbits or a scale factor wider than the type is an error; - the class parameter (integer or float) decides the decode, a scale type that does not match it is refused, and E-scale is refused, as in libhdf5 (no library writes it; it was decoded here unchecked); - minval is the stored bytes zero-extended, as libhdf5 reads it. Codes past the end of the chunk stay an error, as in libhdf5 releases after 2.0 ("Buffer too short"; 2.0 reads past the buffer, cve-2025-2308). Shuffle (filter 2) uses its own parameter as the element size, as libhdf5 does, instead of the dataset's element size; a parameter larger than the chunk leaves the chunk as it is (cve-2025-44905 /Shuffle_float_data_be), and a parameter of 0 is an error. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/filters.rs | 376 ++++++++++++++++---------- 1 file changed, 236 insertions(+), 140 deletions(-) diff --git a/crates/clawhdf5-format/src/filters.rs b/crates/clawhdf5-format/src/filters.rs index 36f2e96..45b54b2 100644 --- a/crates/clawhdf5-format/src/filters.rs +++ b/crates/clawhdf5-format/src/filters.rs @@ -183,7 +183,7 @@ pub(crate) static BUILTIN_FILTERS: &[BuiltinFilter] = &[ BuiltinFilter { id: FILTER_SHUFFLE, name: "shuffle", - decode: |d, c| shuffle_decompress(d, c.element_size), + decode: |d, c| shuffle_decompress(d, shuffle_type_size(c.client_data(), c.element_size)?), encode: Some(|d, c| shuffle_compress(d, c.element_size)), }, BuiltinFilter { @@ -280,23 +280,6 @@ pub(crate) static BUILTIN_FILTERS: &[BuiltinFilter] = &[ }, ]; -/// Decode the HDF5 scale-offset filter (id 6). -/// -/// Supports all three scale-offset variants: -/// - `H5Z_SO_FLOAT_DSCALE` (0): `value = minval + code / 10^D` -/// - `H5Z_SO_FLOAT_ESCALE` (1): `value = minval + code * 2^E` -/// - `H5Z_SO_INT` (2): `value = minval + code` -/// -/// Compressed buffer layout: `minbits` (u32 LE) · `minval_width` (1 byte) -/// · `minval` (`minval_width` bytes) · 8 reserved bytes · MSB-first packed -/// codes (`nelmts * minbits` bits). The all-ones code is reserved for the -/// defined fill value. -/// -/// `cd` is the `H5Zscaleoffset.c` parameter block: `[0]`=scale type, -/// `[1]`=scale factor (decimal digits D for D-scale, binary exponent E for -/// E-scale, interpreted as i32 for negative exponents), `[2]`=element count, -/// `[4]`=element size, `[5]`=signed flag, `[6]`=byte order (1 = big-endian), -/// `[7]`=fill defined, `[8..]`=fill value bits. /// `f64::powi` equivalent that works under `no_std` (no libm/std available). /// Exponentiation by squaring, matching `powi`'s semantics for negative /// exponents via reciprocal. @@ -318,6 +301,32 @@ fn powi_f64(base: f64, mut exp: i32) -> f64 { if neg { 1.0 / result } else { result } } +/// Decode the HDF5 scale-offset filter (id 6) as libhdf5 does +/// (`H5Z__filter_scaleoffset`, reverse direction). +/// +/// - Integers (`H5Z_SO_INT`): `value = minval + code`. +/// - Floats, D-scale (`H5Z_SO_FLOAT_DSCALE`): `value = code / 10^D + min`. +/// - Floats, E-scale: refused, as libhdf5 refuses it ("E-scaling method not +/// supported"); no library writes it. +/// +/// Compressed buffer layout: `minbits` (u32 LE) · the size of `minval` in +/// bytes (1 byte; libhdf5 uses at most 8 of them) · `minval` · packed codes +/// at byte 21, whatever the stored size of `minval` (`buf_offset` is fixed) +/// · MSB-first, `minbits` bits per element. With a fill value defined, the +/// all-ones code of `minbits` bits is the fill value — for `minbits == 0` +/// that is every element. `minbits` equal to the element's full width means +/// the elements are stored as they are (in little-endian order), and an +/// integer scale factor of the full width means the filter left the chunk +/// untouched. +/// +/// `cd` is the `H5Zscaleoffset.c` parameter block: `[0]` scale type, `[1]` +/// scale factor, `[2]` element count, `[3]` class (0 integer, 1 float), +/// `[4]` element size, `[5]` signed, `[6]` byte order (1 = big-endian), `[7]` +/// fill defined, `[8..]` fill value bits. +/// +/// Packed data too short for its codes is an error (libhdf5 2.0 read past +/// the end of the chunk buffer — `cve-2025-2308` — and later releases +/// refuse it: "Buffer too short"). fn scaleoffset_decompress( data: &[u8], cd: &[u32], @@ -326,74 +335,101 @@ fn scaleoffset_decompress( const H5Z_SO_FLOAT_DSCALE: u32 = 0; const H5Z_SO_FLOAT_ESCALE: u32 = 1; const H5Z_SO_INT: u32 = 2; + /// Where the packed codes start (`buf_offset` in `H5Zscaleoffset.c`). + const BUF_OFFSET: usize = 21; + let err = |why: &str| FormatError::ChunkedReadError(format!("scale-offset: {why}")); if cd.len() < 8 { - return Err(FormatError::ChunkedReadError( - "scale-offset: missing filter client data".into(), - )); + return Err(err("missing filter client data")); } let scale_type = cd[0]; - let is_float = scale_type == H5Z_SO_FLOAT_DSCALE || scale_type == H5Z_SO_FLOAT_ESCALE; - if scale_type != H5Z_SO_INT && !is_float { - return Err(FormatError::UnsupportedFilter(FILTER_SCALEOFFSET)); + let is_float = match cd[3] { + 0 => false, + 1 => true, + _ => return Err(err("cannot use C integer datatype for cast")), + }; + if is_float && scale_type != H5Z_SO_FLOAT_DSCALE && scale_type != H5Z_SO_FLOAT_ESCALE + || !is_float && scale_type != H5Z_SO_INT + { + return Err(err("invalid scale type")); + } + if scale_type == H5Z_SO_FLOAT_ESCALE { + return Err(err("E-scaling method not supported")); } let nelmts = cd[2] as usize; let elem_size = cd[4] as usize; - if elem_size == 0 || elem_size > 8 || (is_float && elem_size != 4 && elem_size != 8) { - return Err(FormatError::ChunkedReadError( - "scale-offset: unsupported element size".into(), - )); + let size_ok = if is_float { + matches!(elem_size, 4 | 8) + } else { + matches!(elem_size, 1 | 2 | 4 | 8) + }; + if !size_ok { + return Err(err("cannot use C integer datatype for cast")); + } + let full_bits = elem_size * 8; + // An integer's scale factor is the number of bits kept; all of them + // means the filter stored the chunk as it was. + if !is_float && (cd[1] as i32).max(0) as usize > full_bits { + return Err(err("minimum number of bits exceeds maximum")); + } + if !is_float && cd[1] as i32 == full_bits as i32 { + return Ok(data.to_vec()); } // The decoded output must match the chunk's uncompressed size; reject an // element count that would over-allocate (e.g. minbits == 0 with a huge // nelmts and no packed payload to bound it). let out_bytes = nelmts .checked_mul(elem_size) - .ok_or_else(|| FormatError::ChunkedReadError("scale-offset: size overflow".into()))?; + .ok_or_else(|| err("size overflow"))?; if expected_bytes != 0 && out_bytes > expected_bytes { - return Err(FormatError::ChunkedReadError( - "scale-offset: element count exceeds chunk size".into(), - )); + return Err(err("element count exceeds chunk size")); } let signed = cd[5] == 1; let big_endian = cd[6] == 1; let fill_defined = cd[7] == 1; - // --- header: minbits, then minval, then 8 reserved bytes --- + // --- header: minbits, then the size of minval and minval --- if data.len() < 5 { - return Err(FormatError::ChunkedReadError( - "scale-offset: truncated header".into(), - )); + return Err(err("buffer too short")); } let minbits = u32::from_le_bytes([data[0], data[1], data[2], data[3]]) as usize; - let minval_width = data[4] as usize; - let minval_end = 5 + minval_width; - if data.len() < minval_end { - return Err(FormatError::ChunkedReadError( - "scale-offset: truncated minval".into(), - )); + if minbits > full_bits { + return Err(err("minimum number of bits exceeds size of type")); } - let minval_bytes = &data[5..minval_end]; + let minval_size = usize::from(data[4]).min(8); + let minval_bytes = data + .get(5..5 + minval_size) + .ok_or_else(|| err("buffer too short"))?; + let minval = minval_bytes + .iter() + .rev() + .fold(0u64, |acc, &b| (acc << 8) | u64::from(b)); - // --- unpack the per-element codes (MSB-first), shared by both variants --- - if minbits > 64 { - return Err(FormatError::ChunkedReadError( - "scale-offset: implausible minbits".into(), - )); + // Full precision: the elements follow as they were, little-endian. + if minbits == full_bits { + let raw = data + .get(BUF_OFFSET..) + .and_then(|d| d.get(..out_bytes)) + .ok_or_else(|| err("buffer too short"))?; + let mut out = raw.to_vec(); + if big_endian { + for e in out.chunks_exact_mut(elem_size) { + e.reverse(); + } + } + return Ok(out); } + + // --- unpack the per-element codes (MSB-first) --- let codes: Vec = if minbits == 0 { - // No packed payload: every element equals minval. + // No packed payload: every code is 0. vec![0u64; nelmts] } else { - let packed = data.get(minval_end + 8..).ok_or_else(|| { - FormatError::ChunkedReadError("scale-offset: truncated packed data".into()) - })?; + let packed = data.get(BUF_OFFSET..).unwrap_or(&[]); let need_bits = nelmts .checked_mul(minbits) - .ok_or_else(|| FormatError::ChunkedReadError("scale-offset: size overflow".into()))?; + .ok_or_else(|| err("size overflow"))?; if packed.len() * 8 < need_bits { - return Err(FormatError::ChunkedReadError( - "scale-offset: packed data too short".into(), - )); + return Err(err("packed data too short")); } let mut out = Vec::with_capacity(nelmts); let mut bitpos = 0usize; @@ -408,35 +444,28 @@ fn scaleoffset_decompress( } out }; - // The fill code (all ones) only exists when there are bits to pack. - let has_fill_code = fill_defined && minbits > 0 && minbits < 64; - // Computed for all 1..=64 widths; `1 << 64` would overflow, so saturate. - let fill_code: u64 = if minbits == 0 { - 0 - } else if minbits >= 64 { - u64::MAX - } else { - (1u64 << minbits) - 1 + // With a fill value defined, the all-ones code of `minbits` bits (0 + // when minbits is 0) stands for it. minbits < 64 here. + let fill_code: u64 = (1u64 << minbits) - 1; + let fill_bits = || { + let lo = u64::from(*cd.get(8).unwrap_or(&0)); + let hi = u64::from(*cd.get(9).unwrap_or(&0)); + lo | (hi << 32) }; if is_float { - let is_escale = scale_type == H5Z_SO_FLOAT_ESCALE; let scale_factor = cd[1] as i32; - let minval = read_le_float(minval_bytes, elem_size); + let minval = bits_to_float(minval, elem_size); let fill_value = if fill_defined { - let lo = *cd.get(8).unwrap_or(&0) as u64; - let hi = *cd.get(9).unwrap_or(&0) as u64; - bits_to_float(lo | (hi << 32), elem_size) + bits_to_float(fill_bits(), elem_size) } else { 0.0 }; let values: Vec = codes .iter() .map(|&code| { - if has_fill_code && code == fill_code { + if fill_defined && code == fill_code { fill_value - } else if is_escale { - minval + code as f64 * powi_f64(2.0, scale_factor) } else if elem_size == 4 { // H5Z_scaleoffset_modify_3/4 for `float`: the code is // read as an `int` and everything is single precision, @@ -456,21 +485,19 @@ fn scaleoffset_decompress( .collect(); Ok(write_floats(&values, elem_size, big_endian)) } else { - let minval = read_le_int(minval_bytes, signed); let fill_value: i64 = if fill_defined { - let lo = *cd.get(8).unwrap_or(&0) as u64; - let hi = *cd.get(9).unwrap_or(&0) as u64; - sign_extend(lo | (hi << 32), elem_size, signed) + sign_extend(fill_bits(), elem_size, signed) } else { 0 }; let values: Vec = codes .iter() .map(|&code| { - if has_fill_code && code == fill_code { + if fill_defined && code == fill_code { fill_value } else { - minval.wrapping_add(code as i64) + // `(type)(buf[i] + minval)`: wraps at the element width. + (code.wrapping_add(minval)) as i64 } }) .collect(); @@ -478,21 +505,6 @@ fn scaleoffset_decompress( } } -/// Read a little-endian float of `size` bytes (4 = f32, otherwise f64) as f64. -fn read_le_float(bytes: &[u8], size: usize) -> f64 { - if size == 4 { - let mut b = [0u8; 4]; - let n = bytes.len().min(4); - b[..n].copy_from_slice(&bytes[..n]); - f32::from_le_bytes(b) as f64 - } else { - let mut b = [0u8; 8]; - let n = bytes.len().min(8); - b[..n].copy_from_slice(&bytes[..n]); - f64::from_le_bytes(b) - } -} - /// Interpret the low bits of `raw` as an IEEE float of `size` bytes. fn bits_to_float(raw: u64, size: usize) -> f64 { if size == 4 { @@ -525,16 +537,6 @@ fn write_floats(values: &[f64], elem_size: usize, big_endian: bool) -> Vec { out } -/// Read a little-endian integer of `bytes.len()` bytes, sign-extending when -/// `signed`. Used for the scale-offset `minval` field. -fn read_le_int(bytes: &[u8], signed: bool) -> i64 { - let mut raw: u64 = 0; - for (i, &b) in bytes.iter().enumerate().take(8) { - raw |= (b as u64) << (i * 8); - } - sign_extend(raw, bytes.len().min(8), signed) -} - /// Interpret the low `size` bytes of `raw` as a (possibly signed) integer. fn sign_extend(raw: u64, size: usize, signed: bool) -> i64 { if size == 0 || size >= 8 { @@ -1216,6 +1218,23 @@ fn zstd_compress(data: &[u8], level: u32) -> Result, FormatError> { /// Unshuffle (decompress direction): reconstruct interleaved element bytes. /// On disk: all byte-0s of each element together, then all byte-1s, etc. /// Output: elements in natural order. +/// The element size the shuffle filter works with: its parameter, as +/// libhdf5 uses it (`H5Z__filter_shuffle`), not the dataset's element size. +/// They are the same in every file a library wrote; a corrupt parameter +/// larger than the chunk makes libhdf5 leave the chunk as it is, and so +/// does [`shuffle_decompress`] (`cve-2025-44905`'s `Shuffle_float_data_be`). +/// A zero parameter is an error ("invalid shuffle parameters"); a pipeline +/// without the parameter (never written by libhdf5) uses the element size. +fn shuffle_type_size(cd: &[u32], element_size: usize) -> Result { + match cd { + [] => Ok(element_size), + [0] | [_, _, ..] => Err(FormatError::FilterError( + "invalid shuffle parameters".into(), + )), + [size] => Ok(*size as usize), + } +} + fn shuffle_decompress(data: &[u8], element_size: usize) -> Result, FormatError> { if element_size <= 1 { return Ok(data.to_vec()); @@ -2434,48 +2453,125 @@ mod tests { } } - fn as_f64(bytes: &[u8]) -> Vec { - bytes - .as_chunks::<8>() - .0 - .iter() - .map(|c| f64::from_le_bytes(*c)) - .collect() + /// E-scale: libhdf5 refuses it on read and write ("E-scaling method not + /// supported"); it was decoded here, never checked against anything. + #[test] + fn scaleoffset_float_escale_is_refused_as_in_libhdf5() { + let cd = [1u32, 1, 4, 1, 8, 0, 0, 0]; + let mut raw = vec![2, 0, 0, 0, 8]; + raw.extend_from_slice(&[0; 16]); + raw.push(0x1B); + assert!(scaleoffset_decompress(&raw, &cd, 0).is_err()); } + /// A scale type that does not match the class is refused, as libhdf5 + /// refuses it ("invalid scale type"). #[test] - fn scaleoffset_float_escale_e1() { - // f64 [0.0, 2.0, 4.0, 6.0], E=1 (×2^1=2), fill_defined=0. - // cd: scale_type=1, E=1, nelmts=4, elem_size=8. - let cd = [1u32, 1, 4, 0, 8, 0, 0, 0]; - let raw: &[u8] = &[ - 2, 0, 0, 0, // minbits=2 - 8, // minval_width=8 - 0, 0, 0, 0, 0, 0, 0, 0, // minval=0.0f64 - 0, 0, 0, 0, 0, 0, 0, 0, // 8 reserved bytes - 0x1B, // packed codes: 00 01 10 11 MSB-first - ]; - let got = as_f64(&scaleoffset_decompress(raw, &cd, 0).unwrap()); - assert_eq!(got, vec![0.0, 2.0, 4.0, 6.0]); + fn scaleoffset_scale_type_must_match_the_class() { + let mut raw = vec![2, 0, 0, 0, 8]; + raw.extend_from_slice(&[0; 16]); + raw.push(0x1B); + assert!(scaleoffset_decompress(&raw, &[0, 0, 4, 0, 4, 1, 0, 0], 0).is_err()); + assert!(scaleoffset_decompress(&raw, &[2, 0, 4, 1, 4, 1, 0, 0], 0).is_err()); + assert!(scaleoffset_decompress(&raw, &[2, 0, 4, 7, 4, 1, 0, 0], 0).is_err()); } + /// `cve-2025-44905` `/Scale_offset_short_data_be`, chunk (4, 0): the + /// stored size of `minval` is 0. libhdf5 reads a `minval` of 0 and the + /// packed codes from byte 21 regardless; we read them from byte 13 + /// (5 + size + 8), so the values differed from h5py's. #[test] - fn scaleoffset_float_escale_neg_exp() { - // f64 [0.0, 0.5, 1.0, 1.5], E=-1 (×2^-1=0.5), fill_defined=0. - // cd[1] = 0xFFFF_FFFF which casts to i32 = -1. - let cd = [1u32, 0xFFFF_FFFF, 4, 0, 8, 0, 0, 0]; - let raw: &[u8] = &[ - 2, 0, 0, 0, // minbits=2 - 8, // minval_width=8 - 0, 0, 0, 0, 0, 0, 0, 0, // minval=0.0f64 - 0, 0, 0, 0, 0, 0, 0, 0, // 8 reserved bytes - 0x1B, // packed codes: 00 01 10 11 MSB-first - ]; - let got = as_f64(&scaleoffset_decompress(raw, &cd, 0).unwrap()); - let exp = [0.0f64, 0.5, 1.0, 1.5]; - for (g, e) in got.iter().zip(exp.iter()) { - assert!((g - e).abs() < 1e-9, "got {g} expected {e}"); - } + fn scaleoffset_codes_start_at_byte_21_whatever_the_minval_size() { + // big-endian i16, 12 elements, fill -2 (cd 65534), minbits 3. + let mut cd = vec![2u32, 0, 12, 0, 2, 1, 1, 1, 65534]; + cd.resize(20, 0); + let raw = unhex("0300000000d20e00000000000034000000000000000400000000"); + let got = scaleoffset_decompress(&raw, &cd, 24).unwrap(); + // Codes of 3 bits from byte 21 (04 00 00 00 00): 0, 1, 0, ...; h5py + // reads the chunk's first row as 0, 1, 0. + let want: Vec = vec![0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]; + let want: Vec = want.iter().flat_map(|v| v.to_be_bytes()).collect(); + assert_eq!(got, want); + } + + /// With a fill value defined, libhdf5 compares each code with the + /// all-ones code of `minbits` bits — which for `minbits == 0` is 0, so a + /// chunk with no packed codes reads as all fill values (the compressor + /// writes that for a chunk of nothing but fill values). It read as + /// `minval` here. + #[test] + fn scaleoffset_minbits_zero_with_a_fill_value_is_all_fill() { + let mut cd = vec![2u32, 0, 3, 0, 4, 1, 0, 1, (-7i32) as u32]; + cd.resize(20, 0); + let mut raw = vec![0, 0, 0, 0, 8]; + raw.extend_from_slice(&5i64.to_le_bytes()); + raw.extend_from_slice(&[0; 8]); + assert_eq!( + scaleoffset_decompress(&raw, &cd, 12).unwrap(), + i32_le(&[-7, -7, -7]) + ); + // Without a fill value every element is minval. + cd[7] = 0; + assert_eq!( + scaleoffset_decompress(&raw, &cd, 12).unwrap(), + i32_le(&[5, 5, 5]) + ); + } + + /// `minbits` of the full width stores the elements as they are + /// (little-endian), without `minval`; a full-width integer scale factor + /// means the filter left the chunk untouched. + #[test] + fn scaleoffset_full_width_is_stored_as_is() { + let mut cd = vec![2u32, 0, 2, 0, 2, 1, 1, 0]; + cd.resize(20, 0); + let mut raw = vec![16, 0, 0, 0, 8]; + raw.extend_from_slice(&100i64.to_le_bytes()); + raw.extend_from_slice(&[0; 8]); + raw.extend_from_slice(&[0x34, 0x12, 0xfe, 0xff]); + assert_eq!( + scaleoffset_decompress(&raw, &cd, 4).unwrap(), + [0x12, 0x34, 0xff, 0xfe] + ); + cd[1] = 16; + assert_eq!( + scaleoffset_decompress(&[1, 2, 3, 4], &cd, 4).unwrap(), + [1, 2, 3, 4] + ); + cd[1] = 17; + assert!(scaleoffset_decompress(&[1, 2, 3, 4], &cd, 4).is_err()); + // minbits wider than the type. + cd[1] = 0; + raw[0] = 17; + assert!(scaleoffset_decompress(&raw, &cd, 4).is_err()); + } + + /// The shuffle filter uses its own parameter as the element size, as + /// libhdf5 does; a parameter larger than the chunk leaves the chunk as + /// it is (`cve-2025-44905` `/Shuffle_float_data_be`, whose parameter is + /// 4261347332: h5py and h5dump read the stored bytes unshuffled). + #[test] + fn shuffle_uses_its_parameter() { + let data: Vec = (0..16).collect(); + let shuffled = shuffle_compress(&data, 4).unwrap(); + let pipeline = |cd: Vec| FilterPipeline { + version: 2, + filters: vec![one_filter(FILTER_SHUFFLE, cd)], + }; + // The dataset's element size says 2; the parameter says 4. + assert_eq!( + decompress_chunk(&shuffled, &pipeline(vec![4]), 16, 2).unwrap(), + data + ); + assert_eq!( + decompress_chunk(&shuffled, &pipeline(vec![4_261_347_332]), 16, 4).unwrap(), + shuffled + ); + assert!(decompress_chunk(&shuffled, &pipeline(vec![0]), 16, 4).is_err()); + assert_eq!( + decompress_chunk(&shuffled, &pipeline(vec![]), 16, 4).unwrap(), + data + ); } // --- N-Bit (filter id 5) -------------------------------------------------- From 6b3d003950edd92f838083cd998c16cb3f5d81f1 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:36:01 -0500 Subject: [PATCH 09/20] fix(format): refuse chunk index entries libhdf5 mis-reads - A dataset without filters stores every chunk at the chunk's full size. A chunk its index records at another size was read at that size, with the rest of the chunk left as zeros (cve-2025-44904's Scale_offset_float_data_le: 38- and 37-byte chunks for 48-byte chunks, where HDF5 2.0 fills the rest with whatever its buffer held). It is now refused, as later libhdf5 releases refuse it ("incorrect chunk size returned from index for unfiltered chunk"): chunked_read::list_chunks_for_read, used by every read path. - A v1 B-tree chunk key carries 0 in the element-size dimension. libhdf5 compares that coordinate when it looks a chunk up, so whether it finds a chunk keyed otherwise depends on where the key falls (in cve-2025-44905 /Shuffle_float_data_le, offset 4096, it does not, and h5py reads fill values); we read the chunk. Such a key is now refused. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/chunked_read.rs | 64 +++++++++++++++++-- crates/clawhdf5-format/src/partial_read.rs | 5 +- .../tests/header_validation_interop.rs | 44 +++++++++++++ 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index 70176e2..f9d6567 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -461,6 +461,17 @@ fn collect_chunk_info_inner( file_data[pos + 7], ]); let offsets = read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; + // A chunk's key carries 0 in the element-size dimension. libhdf5 + // compares that coordinate too when it looks a chunk up + // (`H5D__btree_found`), so whether it finds a chunk keyed + // otherwise depends on where the key falls; in `cve-2025-44905` + // `/Shuffle_float_data_le` (offset 4096) it does not, and h5py + // reads fill values there. Such a key is refused here. + if chunk_dimensions.is_some() && offsets.last().is_some_and(|&o| o != 0) { + return Err(FormatError::ChunkedReadError(format!( + "chunk key {offsets:?} has a non-zero element offset" + ))); + } pos += key_size; // Parse child address @@ -820,6 +831,47 @@ pub fn list_chunks( Ok((chunks, chunk_dims)) } +/// [`list_chunks`] for reading the chunks through `pipeline`: a dataset +/// without filters stores every chunk at the chunk's full size, and a chunk +/// the index records at another size is refused, as libhdf5 refuses it +/// ("incorrect chunk size returned from index for unfiltered chunk"). Such +/// a chunk was read at its recorded size, with the rest of the chunk left +/// as zeros or fill values: `cve-2025-44904`'s `Scale_offset_float_data_le` +/// has chunks of 38 and 37 bytes for 48-byte chunks, where HDF5 2.0 reads +/// whatever its buffer held for the missing bytes. +pub fn list_chunks_for_read( + file_data: &[u8], + layout: &DataLayout, + dataspace: &Dataspace, + elem_size: usize, + pipeline: Option<&FilterPipeline>, + offset_size: u8, + length_size: u8, +) -> Result<(Vec, Vec), FormatError> { + let (chunks, chunk_dims) = list_chunks( + file_data, + layout, + dataspace, + elem_size, + offset_size, + length_size, + )?; + if pipeline.is_none_or(|p| p.filters.is_empty()) { + let chunk_bytes = checked_chunk_byte_len(&chunk_dims, elem_size)?; + if let Some(c) = chunks + .iter() + .find(|c| c.address != u64::MAX && c.chunk_size as usize != chunk_bytes) + { + return Err(FormatError::ChunkedReadError(format!( + "incorrect chunk size returned from index for unfiltered chunk at {:?}: \ + {} bytes, expected {chunk_bytes}", + c.offsets, c.chunk_size + ))); + } + } + Ok((chunks, chunk_dims)) +} + pub fn read_chunked_data( file_data: &[u8], layout: &DataLayout, @@ -831,11 +883,12 @@ pub fn read_chunked_data( ) -> Result, FormatError> { check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; - let (chunks, chunk_dims) = list_chunks( + let (chunks, chunk_dims) = list_chunks_for_read( file_data, layout, dataspace, elem_size, + pipeline, offset_size, length_size, )?; @@ -979,11 +1032,12 @@ pub fn read_chunked_data_cached( // lookup is keyed by this dataset's chunk-index address, so another // dataset's index or chunks are never used for this read. let chunks = cache.chunks_for(addr, rank, || { - list_chunks( + list_chunks_for_read( file_data, layout, dataspace, elem_size, + pipeline, offset_size, length_size, ) @@ -1290,11 +1344,12 @@ pub fn read_chunked_data_sweep( // lookup is keyed by this dataset's chunk-index address, so another // dataset's index or chunks are never used for this read. let chunks = cache.chunks_for(addr, rank, || { - list_chunks( + list_chunks_for_read( file_data, layout, dataspace, elem_size, + pipeline, offset_size, length_size, ) @@ -1431,11 +1486,12 @@ pub fn read_chunked_data_indexed( addr, rank, || { - list_chunks( + list_chunks_for_read( file_data, layout, dataspace, elem_size, + pipeline, offset_size, length_size, ) diff --git a/crates/clawhdf5-format/src/partial_read.rs b/crates/clawhdf5-format/src/partial_read.rs index 9865c46..b75887c 100644 --- a/crates/clawhdf5-format/src/partial_read.rs +++ b/crates/clawhdf5-format/src/partial_read.rs @@ -18,7 +18,7 @@ use alloc::{format, vec, vec::Vec}; #[cfg(feature = "std")] use std::string as alloc_or_std; -use crate::chunked_read::{alloc_output, checked_byte_len, list_chunks}; +use crate::chunked_read::{alloc_output, checked_byte_len, list_chunks_for_read}; use crate::data_layout::DataLayout; use crate::data_read::extract_selection_from_buffer; use crate::dataspace::Dataspace; @@ -294,11 +294,12 @@ pub fn read_selection( btree_address: Some(_), .. } => { - let (chunks, chunk_dims) = list_chunks( + let (chunks, chunk_dims) = list_chunks_for_read( file_data, layout, dataspace, elem_size, + pipeline, offset_size, length_size, )?; diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 74dc419..25614c5 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -628,3 +628,47 @@ save("mdc_past_eof", bad) "DSET", ); } + +/// Chunk index entries HDF5 2.0 mis-reads, refused here. An unfiltered +/// chunk the index records at less than the chunk's size (`cve-2025-44904`): +/// HDF5 2.0 fills the rest of the chunk with whatever its buffer held, and +/// later libhdf5 releases refuse it ("incorrect chunk size returned from +/// index for unfiltered chunk"); we read the rest as zeros. A chunk keyed +/// with a non-zero element offset (`cve-2025-44905`): libhdf5's lookup +/// compares that coordinate too, so whether it finds the chunk depends on +/// where the key falls (in `cve-2025-44905` it does not, and h5py reads +/// fill values; in this file it does); we read the chunk. +#[test] +fn chunk_index_entries_libhdf5_misreads_are_refused() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + run_python( + dir.path(), + r#" +good = os.path.join(d, "good.h5") +with h5py.File(good, "w", libver="earliest") as f: + f.create_dataset("d", data=np.arange(100, dtype=" 0 and data[tree + 5] == 0 +key = tree + 8 + 16 # the first chunk's key: size, filter mask, offsets +second = key + 24 + 8 # a key (4 + 4 + 2 x 8 bytes), then a child address +assert struct.unpack_from(" Date: Sat, 26 Sep 2026 10:38:50 -0500 Subject: [PATCH 10/20] conformance: report a cache image libhdf5 cannot load where it does libhdf5 loads a metadata cache image when it first reads metadata (the root group), not at open, so for cve-2025-6269-1..4 and cve-2025-6516 (all corrupt images) h5py opens the file and fails on "/". The probe reported the image's error as an open error, which made those files our-errors; it now records it on the root object, where h5py reports it. File::open still refuses such a file outright: nothing in it can be read. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index c4cf524..9fa9929 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -712,18 +712,35 @@ fn main() { return; } }; - // libhdf5 decodes the superblock extension at open, and loads a - // metadata cache image over the file's own metadata. - let view = match guarded(|| { - clawhdf5_format::superblock_ext::metadata_view(hdf5, &sb).map_err(e) - }) { - Ok(v) => v, + // libhdf5 decodes the superblock extension at open (File::open does + // the same), and loads a metadata cache image over the file's own + // metadata. It loads the image only when it first reads metadata — the + // root group — so a file whose image it cannot load still opens and + // every object fails; File::open refuses such a file outright. The + // probe records the image's error where libhdf5 reports it. + use clawhdf5_format::superblock_ext; + let ext = match guarded(|| superblock_ext::read_superblock_extension(hdf5, &sb).map_err(e)) { + Ok(x) => x, Err(msg) => { top.insert("open_error".into(), Value::String(msg)); println!("{}", Value::Object(top)); return; } }; + let mut image_error = None; + let view = match ext.and_then(|x| x.cache_image) { + None => None, + Some(loc) => match guarded(|| { + superblock_ext::apply_cache_image(hdf5, loc, sb.offset_size, sb.length_size) + .map_err(e) + }) { + Ok(v) => Some(v), + Err(msg) => { + image_error = Some(msg); + None + } + }, + }; let hdf5: &[u8] = view.as_deref().unwrap_or(hdf5); top.insert("superblock_version".into(), json!(sb.version)); let ctx = Ctx { @@ -752,6 +769,9 @@ fn main() { let mut rec = Map::new(); rec.insert("path".into(), Value::String(p.clone())); let r = guarded(|| { + if let Some(msg) = &image_error { + return Err(msg.clone()); + } let h = ctx.header(addr)?; Ok(h) }); From 67958b08d9f76c2e2d17f4b5c34fc869229ca51f Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:39:25 -0500 Subject: [PATCH 11/20] conformance: list the corrupt objects HDF5 2.0 reads through a bug Four of the remaining our-errors are objects the reference (h5py 3.16 / HDF5 2.0) reads only through a libhdf5 bug, and clawhdf5 refuses: cve-2025-2308 (scale-offset codes past the end of the chunk), cve-2025-44904 (short unfiltered chunks), bad_nbit_parms_walk.h5 (an N-Bit parameter list one value short; libhdf5's own test_filter_bad_params now requires the read to fail) and cve-2025-44905 /Shuffle_float_data_le (a chunk key libhdf5's lookup misses, reading fill values). report.py lists them under Known not-our-bug and counts them in the summary; they stay our-errors in the class counts. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/report.py | 48 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/conformance/report.py b/conformance/report.py index dc82413..3331898 100644 --- a/conformance/report.py +++ b/conformance/report.py @@ -98,13 +98,40 @@ def is_h5py_be_vlen(i): and ">" in (i.get("ours_dtype") or "")) +# Objects the reference (h5py 3.16 / HDF5 2.0) reads only because of an +# HDF5 2.0 bug, and that clawhdf5 refuses: each one reads past a buffer or +# returns bytes the file does not hold, and libhdf5's develop branch refuses the +# first three. (file, object) -> why. Checked 2026-09-26 against HDF5 2.0.0 +# and HDFGroup/hdf5 develop sources; see docs/known-issues.md. +LIBHDF5_BUGS = { + ("cve_hdf5/cvefiles/cve-2025-2308.h5", "/Scale_offset_long_long_data_le"): + "scale-offset codes run past the end of the chunk: HDF5 2.0 reads past its buffer; " + "libhdf5's develop branch refuses the chunk (\"Buffer too short\")", + ("cve_hdf5/cvefiles/cve-2025-44904.h5", "/Scale_offset_float_data_le"): + "unfiltered chunks of 38 and 37 bytes for 48-byte chunks: HDF5 2.0 fills the rest with " + "whatever its buffer held; libhdf5's develop branch refuses them (\"incorrect chunk size returned " + "from index for unfiltered chunk\")", + ("hdf5/test/testfiles/bad_nbit_parms_walk.h5", "/Nbit_int_data_le"): + "an N-Bit parameter list one value short: HDF5 2.0 reads past the list; libhdf5's own " + "test (`test_filter_bad_params`, test/dsets.c) now requires the read to fail", + ("cve_hdf5/cvefiles/cve-2025-44905.h5", "/Shuffle_float_data_le"): + "a chunk B-tree key with element offset 4096: libhdf5's lookup misses the chunk and " + "returns fill values for data the file holds; clawhdf5 refuses the key", +} + + +def is_libhdf5_bug(rel, i): + return i["kind"] == "our-error" and any( + f == rel and i["detail"].startswith(obj + ":") for (f, obj) in LIBHDF5_BUGS) + + known = collections.defaultdict(list) for r in rows: - if r["class"] != "mismatch": - continue iss = issues.get(r["file"], []) - if iss and all(is_h5py_be_vlen(i) for i in iss): + if r["class"] == "mismatch" and iss and all(is_h5py_be_vlen(i) for i in iss): known["h5py-be-vlen"].append(r["file"]) + if r["class"] == "our-error" and iss and all(is_libhdf5_bug(r["file"], i) for i in iss): + known["libhdf5-2.0"].append(r["file"]) # --- the CVE corpus: clawhdf5 vs h5dump vs h5py ------------------------------ @@ -230,9 +257,13 @@ for c in sorted(by_corpus): w(f"| {c} | {sum(cnt.values())} | " + " | ".join(str(cnt.get(k, 0)) for k in CLASSES) + " |") w(f"| **all** | **{len(rows)}** | " + " | ".join(f"**{total.get(k, 0)}**" for k in CLASSES) + " |") w("") -n_known = sum(len(v) for v in known.values()) -if n_known: - w(f"{n_known} of the {total.get('mismatch', 0)} mismatches are a known h5py bug, not ours (see *Known not-our-bug*).") +if known["h5py-be-vlen"]: + w(f"{len(known['h5py-be-vlen'])} of the {total.get('mismatch', 0)} mismatches are a known h5py bug, " + "not ours (see *Known not-our-bug*).") + w("") +if known["libhdf5-2.0"]: + w(f"{len(known['libhdf5-2.0'])} of the {total.get('our-error', 0)} our-errors are corrupt data that " + "HDF5 2.0 reads only through a bug and clawhdf5 refuses (see *Known not-our-bug*).") w("") w("Corpora (fetched by `conformance/fetch-corpus.sh` into the gitignored `conformance/.cache/`):") w("") @@ -311,6 +342,11 @@ if res["incomparable"]: w(" (FP8 -> float16, bfloat16 -> float32, x87 long double -> float128) the values are not") w(" compared (shape and presence still are): " + ", ".join(f"{k} ({n}x)" for k, n in res["incomparable"]) + ".") +w("- **Corrupt data HDF5 2.0 reads through a bug.** clawhdf5 refuses these objects; h5py 3.16 /") +w(" HDF5 2.0 returns values for them that the file does not hold:") +for (f, obj), why in sorted(LIBHDF5_BUGS.items()): + here = "" if f in known["libhdf5-2.0"] else " (not an our-error in this run)" + w(f" - `{f}` `{obj}`: {why}{here}.") w("- **References** are compared by presence only (`R`), not by target.") w("") if res.get("ref_only_errors"): From 378afa1584164109fc5c5651bb28cc07dcb9ed1b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 10:40:18 -0500 Subject: [PATCH 12/20] docs: changelog and known issues for the remaining conformance errors Conformance on tank, conformance/run.sh --no-fetch (2026-09-26): 597 of 697 ok, 6 our-errors (4 corrupt objects HDF5 2.0 reads through a bug, the Blosc2 and ZFP filters), 2 mismatches (the known h5py big-endian VL bug). Closes the known-issues entries for metadata cache images, cve-2024-32624, cve-2020-10810/10812, and unfiltered chunks of the wrong size; the N-Bit / 64-bit scale-offset entry is recorded as not our bug. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 54 ++++++++++++++++++++++++++++++++++++++++++++ docs/known-issues.md | 37 +++++++++++++++++++++++++----- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29804fc..c13a8da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,60 @@ ## Unreleased +### Remaining conformance errors (2026-09-26) +Conformance on tank, `conformance/run.sh --no-fetch`: 597 of 697 files +ok (575 before). Of the 6 our-errors left, 4 are corrupt data HDF5 2.0 +reads only through a bug (listed in `CONFORMANCE.md`), 2 are the Blosc2 and +ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. +- **Metadata cache images are read.** A file written with a metadata cache + image keeps its metadata cache entries in an image block the superblock + extension points at, and libhdf5 reads them in place of the file's own + bytes; in `h5clear_mdc_image.h5` the root group exists only there, and + every reader failed with `InvalidObjectHeaderVersion(0)`. `File`, + `MmapFile` and `LazyFile` (and `h5rs`) now apply the image at open + (`clawhdf5_format::superblock_ext`), with libhdf5's checks. A file whose + image libhdf5 cannot load opens in libhdf5 but nothing in it can be read; + `File::open` refuses it. +- **The superblock extension is decoded at open, as libhdf5 does:** a File + Space Info or Metadata Cache Image message libhdf5 cannot decode makes the + open fail (`cve-2020-10810`, `cve-2020-10812` were opened). + `FormatError::InvalidSuperblockExtension`, `InvalidCacheImage`. +- **Dataset storage libhdf5 refuses at open is refused at open** + (`FormatError::InvalidDatasetStorage`, `data_read::check_dataset_storage`): + an element count times element size that overflows (`cve-2024-32624` + `/Dset_OBJREF` opened and reported its shape), contiguous storage past the + end of the file, compact data of the wrong size. An empty contiguous + dataset at a defined address, which clawhdf5 up to v2.7.0 wrote, still + opens. +- **Wrong or missing data fixed:** + - a simple dataspace of rank 0 holds one element (it held 0; + `cve-2020-18494`), and contiguous storage larger than the dataset reads + (`cve-2024-32623`, `cve-2025-2309`; libhdf5 ignores the excess); + - scale-offset: the packed codes start at byte 21 whatever size the chunk + records for `minval`, a chunk with `minbits` 0 and a fill value is all + fill values, full-width `minbits` stores the elements as they are + (these decoded differently from libhdf5); E-scale is refused, as in + libhdf5; codes past the end of the chunk stay an error + (`cve-2025-2308`, where HDF5 2.0 reads past its buffer); + - shuffle uses its own parameter as the element size, as libhdf5 does + (`cve-2025-44905`); + - an unfiltered chunk the index records at other than the chunk's size is + refused (it read with zeros for the missing bytes; `cve-2025-44904`), + as is a chunk B-tree key with a non-zero element offset. +- **Refused as libhdf5 refuses them:** a v1 group with an empty link name + fails its listing (`FormatError::InvalidLinkName`; lookups still work, + `cve-2021-46244`); dataspaces with more than 32 dimensions, a rank on a + scalar or null dataspace, or a dimension over its maximum + (`FormatError::InvalidDataspace`). +- `ObjectHeader::object_class` classifies a header as libhdf5 does (a + dataset needs a datatype *and* a dataspace). +- Conformance harness: user-defined links were listed as objects by the + reference, unopenable objects were not deduplicated, nested array types + were hashed wrong (`tarray3.h5`), and the attributes of objects h5py + cannot open were compared; all fixed. `CONFORMANCE.md` lists the corrupt + objects HDF5 2.0 reads through a bug (`bad_nbit_parms_walk.h5` among + them: libhdf5's own test now requires that read to fail). + ### Concurrent reads (2026-09-26) - **Full reads of chunked datasets scale with threads again when rayon's pool has one thread.** Each full read handed its chunks to rayon to diff --git a/docs/known-issues.md b/docs/known-issues.md index 2657cdf..b7c471a 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -202,9 +202,25 @@ fill-value item that did is fixed). its datatype message stores (12 with 4-byte offsets), and the global heap is read with libhdf5's header padding (`crates/clawhdf5/tests/vl_offset4_interop.rs`). - - Metadata cache images are not supported. + - Metadata cache images are not supported. **Fixed 2026-09-26:** the + image is applied at open, as libhdf5 loads it over the file's metadata + (`clawhdf5_format::superblock_ext`); `h5clear_mdc_image.h5` reads + (`crates/clawhdf5/tests/metadata_cache_image.rs`). A file whose image + libhdf5 cannot load (`cve-2025-6269-*`, `cve-2025-6516`) opens in + libhdf5 with nothing readable in it; `File::open` refuses it. - x87 long double and binary128 are refused. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail. + **Not our bug (checked 2026-09-26):** both are corrupt files HDF5 2.0 + reads only by reading past a buffer. `cve-2025-2308`'s + `/Scale_offset_long_long_data_le` has scale-offset codes that run past + the end of the chunk (libhdf5's develop branch refuses it, "Buffer too + short"); `bad_nbit_parms_walk.h5` has an N-Bit parameter list one value + short (libhdf5's own `test_filter_bad_params` in `test/dsets.c` now + requires that read to fail). We refuse both; `CONFORMANCE.md` lists them + under *Known not-our-bug*. Scale-offset did decode three cases + differently from libhdf5 (codes after a `minval` of recorded size other + than 8, `minbits` 0 with a fill value, full-width `minbits`): fixed + 2026-09-26. - **Filters:** blosc, blosc2, bitshuffle, bzip2, LZF and zfp are not implemented. **Fixed 2026-09-26** for LZF (default-on `lzf` feature), bitshuffle, bzip2 and Blosc 1 (`bitshuffle`, `bzip2`, `blosc`, or @@ -219,7 +235,11 @@ fill-value item that did is fixed). read with zeros for the missing bytes** (any filter; found reviewing the plugin filters). **Fixed 2026-09-26:** it is an error naming the chunk. A corrupt chunk must never read as zeros. Unfiltered chunks are read at their stored - size and are not checked this way. + size and are not checked this way. **Fixed 2026-09-26** for unfiltered + chunks too: in a dataset without filters a chunk the index records at + other than the chunk's size is refused, as libhdf5's develop branch + refuses it (`cve-2025-44904`, where HDF5 2.0 fills the rest from its + buffer). - **Crash:** a hostile Blosc chunk (frame size below its header) panicked in builds with overflow checks. **Fixed 2026-09-26**; the new decoders are fuzzed in the unit tests. @@ -232,13 +252,18 @@ fill-value item that did is fixed). refused. 17 of the 18 now fail as in libhdf5 (conformance on tank, `conformance/run.sh --no-fetch`, 2026-09-26: 571 of 697 ok). Still read where libhdf5 refuses: - - `cve-2024-32624.h5` `/Dset_OBJREF`: a dataspace whose storage size + - ~~`cve-2024-32624.h5` `/Dset_OBJREF`: a dataspace whose storage size overflows 64 bits. `File::dataset` and `shape()` succeed (libhdf5 - refuses at open); reading the values fails. - - `cve-2020-10810.h5`, `cve-2020-10812.h5` (whole files libhdf5 cannot + refuses at open); reading the values fails.~~ **Fixed 2026-09-26:** + `File::dataset` (and `MmapFile`, `LazyFile`) refuse it at open + (`FormatError::InvalidDatasetStorage`), as they do contiguous storage + past the end of the file. + - ~~`cve-2020-10810.h5`, `cve-2020-10812.h5` (whole files libhdf5 cannot open, not among the 18): libhdf5 decodes the superblock extension's File Space Info and metadata-cache-image messages at open and refuses these - files; we do not decode those messages at open. + files; we do not decode those messages at open.~~ **Fixed 2026-09-26:** + the superblock extension is decoded at open with libhdf5's checks, and + both files are refused. - Deliberately not refused, because clawhdf5 up to v2.7.0 wrote them: a float sign bit position outside the type, and a size-0 string type. - Not refused because current libhdf5 reads it though HDF5 2.0.0 From 742ed4dfb81a488f6269c7c81ddac8ba5e43efed Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:35:13 -0500 Subject: [PATCH 13/20] fix(format): read a v1 chunk B-tree where libhdf5's lookup finds chunks libhdf5 does not walk the chunk B-tree to read a dataset: it looks each chunk up (H5B_find with H5D__btree_cmp3 and H5D__btree_found), asking for the element-size coordinate as 0. collect_chunk_info_checked now parses the tree with its keys and returns each stored chunk only when that lookup, replayed over the scaled keys, finds it. A key with a non-zero element-size coordinate is therefore found in a 1-D dataset (cmp3 compares only the first coordinate there, and found compares with <=) and missed in a dataset of rank 2 or more, which reads fill values. The previous commit refused every such key, which refused 1-D files libhdf5 reads correctly; before that, the rank-2 case read the chunk's data where h5py reads fill values (cve-2025-44905 /Shuffle_float_data_le, now identical to h5py, so it leaves the conformance report's list of libhdf5 bugs). Test: chunk_keys_with_an_element_offset_read_as_libhdf5_reads_them compares 1-D and 2-D files against h5py's values. It fails on the previous commit (the 1-D file is refused) and with the refusal removed (the 2-D file reads 0..23 where h5py reads fill values). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 11 +- conformance/report.py | 7 +- crates/clawhdf5-format/src/chunked_read.rs | 355 +++++++++++++----- .../tests/header_validation_interop.rs | 74 +++- 4 files changed, 322 insertions(+), 125 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c13a8da..8cc4248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,15 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. - shuffle uses its own parameter as the element size, as libhdf5 does (`cve-2025-44905`); - an unfiltered chunk the index records at other than the chunk's size is - refused (it read with zeros for the missing bytes; `cve-2025-44904`), - as is a chunk B-tree key with a non-zero element offset. + refused (it read with zeros for the missing bytes; `cve-2025-44904`); + - a v1 B-tree chunk index is read as libhdf5 reads it: each chunk is + looked up the way `H5B_find` / `H5D__btree_cmp3` / `H5D__btree_found` + look it up, and a chunk that lookup does not find reads as fill values. + A key with a non-zero element-size coordinate is found in a 1-D dataset + and not in one of rank 2 or more (`cve-2025-44905` + `/Shuffle_float_data_le`, which read the chunk's data where h5py reads + fill values); an interim fix refused every such key, including 1-D + files libhdf5 reads correctly. - **Refused as libhdf5 refuses them:** a v1 group with an empty link name fails its listing (`FormatError::InvalidLinkName`; lookups still work, `cve-2021-46244`); dataspaces with more than 32 dimensions, a rank on a diff --git a/conformance/report.py b/conformance/report.py index 3331898..85ad0aa 100644 --- a/conformance/report.py +++ b/conformance/report.py @@ -100,8 +100,8 @@ def is_h5py_be_vlen(i): # Objects the reference (h5py 3.16 / HDF5 2.0) reads only because of an # HDF5 2.0 bug, and that clawhdf5 refuses: each one reads past a buffer or -# returns bytes the file does not hold, and libhdf5's develop branch refuses the -# first three. (file, object) -> why. Checked 2026-09-26 against HDF5 2.0.0 +# returns bytes the file does not hold, and libhdf5's develop branch refuses all +# three. (file, object) -> why. Checked 2026-09-26 against HDF5 2.0.0 # and HDFGroup/hdf5 develop sources; see docs/known-issues.md. LIBHDF5_BUGS = { ("cve_hdf5/cvefiles/cve-2025-2308.h5", "/Scale_offset_long_long_data_le"): @@ -114,9 +114,6 @@ LIBHDF5_BUGS = { ("hdf5/test/testfiles/bad_nbit_parms_walk.h5", "/Nbit_int_data_le"): "an N-Bit parameter list one value short: HDF5 2.0 reads past the list; libhdf5's own " "test (`test_filter_bad_params`, test/dsets.c) now requires the read to fail", - ("cve_hdf5/cvefiles/cve-2025-44905.h5", "/Shuffle_float_data_le"): - "a chunk B-tree key with element offset 4096: libhdf5's lookup misses the chunk and " - "returns fill values for data the file holds; clawhdf5 refuses the key", } diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index f9d6567..e943a03 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -316,26 +316,41 @@ pub fn collect_chunk_info( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - collect_chunk_info_inner( + let _ = length_size; + let mut chunks = Vec::new(); + parse_chunk_node( file_data, btree_address, ndims, None, offset_size, - length_size, 0, - ) + &mut chunks, + )?; + Ok(chunks) } -/// [`collect_chunk_info`] for a layout with these `chunk_dimensions` (the -/// layout message's list, element size last), checking every key of the -/// B-tree as libhdf5 does (`H5D__btree_decode_key`): each coordinate offset -/// must be a multiple of its chunk dimension. That includes the keys that -/// only bound a node (internal-node keys and each node's final key), which -/// is where a corrupt chunk dimension shows when the chunks themselves all -/// start at offset 0 in that dimension (`cve-2018-11205`). A key that fails -/// ("bad coordinate offset") means a corrupt index or chunk dimension; the -/// chunks were read at the wrong place, or the dataset read as fill values. +/// The chunks of a v1 B-tree chunk index as libhdf5 reads them, for a +/// layout with these `chunk_dimensions` (the layout message's list, element +/// size last). +/// +/// Every key of the B-tree is checked as libhdf5 checks it +/// (`H5D__btree_decode_key`): each coordinate offset must be a multiple of +/// its chunk dimension. That includes the keys that only bound a node +/// (internal-node keys and each node's final key), which is where a +/// corrupt chunk dimension shows when the chunks themselves all start at +/// offset 0 in that dimension (`cve-2018-11205`). A key that fails ("bad +/// coordinate offset") means a corrupt index or chunk dimension. +/// +/// libhdf5 does not read a chunk by walking the tree: it looks each chunk +/// up (`H5B_find` with `H5D__btree_cmp3` and `H5D__btree_found`), comparing +/// the element-size coordinate too, which it asks for as 0. So a chunk is +/// returned only where that lookup finds it: a key whose element-size +/// coordinate is not 0 is found in a 1-D dataset (the comparison looks at +/// that coordinate only against the next key) but not in a dataset of rank +/// 2 or more (`cve-2025-44905` `/Shuffle_float_data_le`), which then reads +/// as fill values, and a tree whose keys are out of order loses the chunks +/// libhdf5's binary search misses. pub fn collect_chunk_info_checked( file_data: &[u8], btree_address: u64, @@ -343,15 +358,141 @@ pub fn collect_chunk_info_checked( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - collect_chunk_info_inner( + let _ = length_size; + let ndims = chunk_dimensions.len(); + if ndims == 0 { + return Err(FormatError::ChunkedReadError( + "chunk layout has no dimensions".into(), + )); + } + let mut stored = Vec::new(); + let mut root = parse_chunk_node( file_data, btree_address, - chunk_dimensions.len(), + ndims, Some(chunk_dimensions), offset_size, - length_size, 0, - ) + &mut stored, + )?; + // Keys were checked to be multiples of non-zero dimensions. + root.scale_keys(chunk_dimensions); + // Look every stored chunk's position up. A lookup finds a chunk only at + // that chunk's own position, so each is returned at most once. + let mut returned = vec![false; stored.len()]; + let mut wanted = vec![0u64; ndims]; + for chunk in &stored { + for (w, (&o, &d)) in wanted + .iter_mut() + .zip(chunk.offsets.iter().zip(chunk_dimensions)) + { + *w = o / u64::from(d); + } + wanted[ndims - 1] = 0; + if let Some(i) = root.find(&wanted) { + returned[i] = true; + } + } + Ok(stored + .into_iter() + .zip(returned) + .filter_map(|(c, r)| r.then_some(c)) + .collect()) +} + +/// A node of a v1 B-tree chunk index: its `n + 1` keys (`ndims` +/// coordinates each, flattened; byte offsets as stored, or scaled by the +/// chunk dimensions after [`ChunkNode::scale_keys`]) and its `n` children, +/// a leaf's as indices into the list of stored chunks. +struct ChunkNode { + ndims: usize, + keys: Vec, + children: ChunkChildren, +} + +enum ChunkChildren { + Nodes(Vec), + Chunks(Vec), +} + +impl ChunkNode { + fn key(&self, i: usize) -> &[u64] { + &self.keys[i * self.ndims..(i + 1) * self.ndims] + } + + fn len(&self) -> usize { + match &self.children { + ChunkChildren::Nodes(n) => n.len(), + ChunkChildren::Chunks(c) => c.len(), + } + } + + fn scale_keys(&mut self, dims: &[u32]) { + for (k, &d) in self.keys.iter_mut().zip(dims.iter().cycle()) { + *k /= u64::from(d); + } + if let ChunkChildren::Nodes(nodes) = &mut self.children { + for n in nodes { + n.scale_keys(dims); + } + } + } + + /// `H5B_find_helper` over scaled keys: binary search for the child + /// whose keys bracket `scaled`, then `H5D__btree_found` at the leaf. + /// Returns the index of the chunk found. + fn find(&self, scaled: &[u64]) -> Option { + let (mut lt, mut rt) = (0, self.len()); + let mut idx = 0; + let mut cmp = core::cmp::Ordering::Greater; + while lt < rt && cmp != core::cmp::Ordering::Equal { + idx = (lt + rt) / 2; + cmp = btree_cmp3(self.key(idx), scaled, self.key(idx + 1)); + if cmp == core::cmp::Ordering::Less { + rt = idx; + } else { + lt = idx + 1; + } + } + if cmp != core::cmp::Ordering::Equal { + return None; + } + match &self.children { + ChunkChildren::Nodes(nodes) => nodes[idx].find(scaled), + ChunkChildren::Chunks(chunks) => { + // "Is this *really* the requested chunk?" + let lt_key = self.key(idx); + let found = scaled + .iter() + .zip(lt_key) + .all(|(&s, &k)| s < k.wrapping_add(1)); + found.then_some(chunks[idx]) + } + } + } +} + +/// `H5D__btree_cmp3`: where `scaled` falls against a child's left and +/// right keys. `Less` is left of the child, `Greater` right of it. With a +/// rank-1 dataset (two coordinates, element size last) libhdf5 compares +/// only the first coordinate, and the second against the right key. +fn btree_cmp3(lt: &[u64], scaled: &[u64], rt: &[u64]) -> core::cmp::Ordering { + use core::cmp::Ordering; + if scaled.len() == 2 { + if scaled[0] > rt[0] || (scaled[0] == rt[0] && scaled[1] >= rt[1]) { + Ordering::Greater + } else if scaled[0] < lt[0] { + Ordering::Less + } else { + Ordering::Equal + } + } else if scaled >= rt { + Ordering::Greater + } else if scaled < lt { + Ordering::Less + } else { + Ordering::Equal + } } /// Check one v1 B-tree chunk key's offsets (see @@ -368,24 +509,25 @@ fn check_key_offsets(offsets: &[u64], chunk_dimensions: &[u32]) -> Result<(), Fo } /// Read the `ndims` 8-byte offsets of the chunk key at `pos` (after its -/// chunk size and filter mask) and check them when `chunk_dimensions` is -/// given. +/// chunk size and filter mask) into `out`, checking them when +/// `chunk_dimensions` is given. fn read_key_offsets( file_data: &[u8], pos: usize, ndims: usize, chunk_dimensions: Option<&[u32]>, -) -> Result, FormatError> { - let mut offsets = Vec::with_capacity(ndims); + out: &mut Vec, +) -> Result<(), FormatError> { + let start = out.len(); let mut kp = pos + 8; for _ in 0..ndims { - offsets.push(read_offset(file_data, kp, CHUNK_KEY_OFFSET_SIZE)?); + out.push(read_offset(file_data, kp, CHUNK_KEY_OFFSET_SIZE)?); kp += CHUNK_KEY_OFFSET_SIZE as usize; } if let Some(dims) = chunk_dimensions { - check_key_offsets(&offsets, dims)?; + check_key_offsets(&out[start..], dims)?; } - Ok(offsets) + Ok(()) } /// Width of each chunk offset in a v1 chunk B-tree key, independent of the @@ -396,15 +538,17 @@ const CHUNK_KEY_OFFSET_SIZE: u8 = 8; /// protection), matching `btree_v1.rs`'s `MAX_BTREE_DEPTH`. const MAX_CHUNK_BTREE_DEPTH: usize = 64; -fn collect_chunk_info_inner( +/// Parse the v1 B-tree chunk index node at `btree_address` and its +/// subtree, appending its chunks to `stored` in tree order. +fn parse_chunk_node( file_data: &[u8], btree_address: u64, ndims: usize, chunk_dimensions: Option<&[u32]>, offset_size: u8, - _length_size: u8, depth: usize, -) -> Result, FormatError> { + stored: &mut Vec, +) -> Result { if depth > MAX_CHUNK_BTREE_DEPTH { return Err(FormatError::NestingDepthExceeded); } @@ -439,85 +583,68 @@ fn collect_chunk_info_inner( .and_then(|n| n.checked_add(8)) .ok_or_else(|| FormatError::ChunkedReadError("chunk key too large".into()))?; - if node_level == 0 { - // Leaf node: keys and children interleaved - // key[0], child[0], key[1], child[1], ..., key[N-1], child[N-1], key[N] - let needed = entries_used * (key_size + os) + key_size; - ensure_len(file_data, pos, needed)?; + // key[0], child[0], key[1], child[1], ..., key[N-1], child[N-1], key[N] + let needed = entries_used * (key_size + os) + key_size; + ensure_len(file_data, pos, needed)?; - let mut chunks = Vec::with_capacity(entries_used); - for _ in 0..entries_used { - // Parse key - let chunk_size = u32::from_le_bytes([ - file_data[pos], - file_data[pos + 1], - file_data[pos + 2], - file_data[pos + 3], - ]); - let filter_mask = u32::from_le_bytes([ - file_data[pos + 4], - file_data[pos + 5], - file_data[pos + 6], - file_data[pos + 7], - ]); - let offsets = read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; - // A chunk's key carries 0 in the element-size dimension. libhdf5 - // compares that coordinate too when it looks a chunk up - // (`H5D__btree_found`), so whether it finds a chunk keyed - // otherwise depends on where the key falls; in `cve-2025-44905` - // `/Shuffle_float_data_le` (offset 4096) it does not, and h5py - // reads fill values there. Such a key is refused here. - if chunk_dimensions.is_some() && offsets.last().is_some_and(|&o| o != 0) { - return Err(FormatError::ChunkedReadError(format!( - "chunk key {offsets:?} has a non-zero element offset" - ))); - } - pos += key_size; - - // Parse child address - let address = read_offset(file_data, pos, offset_size)?; - pos += os; - - chunks.push(ChunkInfo { + let mut keys = Vec::with_capacity((entries_used + 1) * ndims); + let mut chunks = Vec::new(); + let mut child_addrs = Vec::new(); + for _ in 0..entries_used { + let chunk_size = u32::from_le_bytes([ + file_data[pos], + file_data[pos + 1], + file_data[pos + 2], + file_data[pos + 3], + ]); + let filter_mask = u32::from_le_bytes([ + file_data[pos + 4], + file_data[pos + 5], + file_data[pos + 6], + file_data[pos + 7], + ]); + let k = keys.len(); + read_key_offsets(file_data, pos, ndims, chunk_dimensions, &mut keys)?; + pos += key_size; + let address = read_offset(file_data, pos, offset_size)?; + pos += os; + if node_level == 0 { + chunks.push(stored.len()); + stored.push(ChunkInfo { chunk_size, filter_mask, - offsets, + offsets: keys[k..].to_vec(), address, }); + } else { + child_addrs.push(address); } - // The final key only bounds the node; libhdf5 still checks it. - read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; - Ok(chunks) + } + // The final key only bounds the node; libhdf5 still checks it. + read_key_offsets(file_data, pos, ndims, chunk_dimensions, &mut keys)?; + + let children = if node_level == 0 { + ChunkChildren::Chunks(chunks) } else { - // Internal node: recurse into children - let needed = entries_used * (key_size + os) + key_size; - ensure_len(file_data, pos, needed)?; - - let mut child_addrs = Vec::with_capacity(entries_used); - for _ in 0..entries_used { - read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; - pos += key_size; - let child_addr = read_offset(file_data, pos, offset_size)?; - child_addrs.push(child_addr); - pos += os; - } - read_key_offsets(file_data, pos, ndims, chunk_dimensions)?; - - let mut all_chunks = Vec::new(); + let mut nodes = Vec::with_capacity(child_addrs.len()); for child_addr in child_addrs { - let child_chunks = collect_chunk_info_inner( + nodes.push(parse_chunk_node( file_data, child_addr, ndims, chunk_dimensions, offset_size, - _length_size, depth + 1, - )?; - all_chunks.extend(child_chunks); + stored, + )?); } - Ok(all_chunks) - } + ChunkChildren::Nodes(nodes) + }; + Ok(ChunkNode { + ndims, + keys, + children, + }) } /// Generate ChunkInfo entries for an implicit index (v4 index type 2). @@ -1754,6 +1881,25 @@ mod tests { /// Build a B-tree v1 type 1 leaf node with given chunk infos. fn build_chunk_btree_leaf(chunks: &[ChunkInfo], ndims: usize, offset_size: u8) -> Vec { + build_chunk_btree_leaf_to(chunks, &vec![0; ndims], offset_size) + } + + /// A leaf whose final key is the one libhdf5 writes past the last chunk: + /// each coordinate of the last chunk plus its chunk dimension (the + /// element size last). + fn build_chunk_btree_leaf_dims(chunks: &[ChunkInfo], dims: &[u32], offset_size: u8) -> Vec { + let last = &chunks.last().expect("a chunk").offsets; + let end: Vec = dims + .iter() + .enumerate() + .map(|(d, &c)| last.get(d).copied().unwrap_or(0) + u64::from(c)) + .collect(); + build_chunk_btree_leaf_to(chunks, &end, offset_size) + } + + /// A leaf holding `chunks`, with final key `end`. + fn build_chunk_btree_leaf_to(chunks: &[ChunkInfo], end: &[u64], offset_size: u8) -> Vec { + let ndims = end.len(); let _os = offset_size as usize; let entries_used = chunks.len() as u16; let mut buf = Vec::new(); @@ -1795,8 +1941,8 @@ mod tests { // checks; 0 always is) buf.extend_from_slice(&0u32.to_le_bytes()); // chunk_size buf.extend_from_slice(&0u32.to_le_bytes()); // filter_mask - for _ in 0..ndims { - write_offset(&mut buf, 0, 8); + for &e in end { + write_offset(&mut buf, e, 8); } buf @@ -1812,8 +1958,11 @@ mod tests { offsets, address, }; - let good = - build_chunk_btree_leaf(&[chunk(vec![0, 0], 0x100), chunk(vec![10, 0], 0x200)], 2, 8); + let good = build_chunk_btree_leaf_dims( + &[chunk(vec![0, 0], 0x100), chunk(vec![10, 0], 0x200)], + &[10, 8], + 8, + ); assert_eq!( collect_chunk_info_checked(&good, 0, &[10, 8], 8, 8) .unwrap() @@ -2044,7 +2193,6 @@ mod tests { ) -> (Vec, DataLayout, Dataspace) { let os: u8 = 8; let elem_size = 8usize; - let ndims = 2; // rank(1) + 1 let total = values.len(); // Place chunk data starting at offset 0x2000 @@ -2075,12 +2223,13 @@ mod tests { } // Build B-tree at offset 0x100 - let btree = build_chunk_btree_leaf(&chunk_infos, ndims, os); + let dims = [chunk_size_elems as u32, elem_size as u32]; + let btree = build_chunk_btree_leaf_dims(&chunk_infos, &dims, os); let btree_addr = 0x100usize; file_data[btree_addr..btree_addr + btree.len()].copy_from_slice(&btree); let layout = DataLayout::Chunked { - chunk_dimensions: vec![chunk_size_elems as u32, elem_size as u32], + chunk_dimensions: dims.to_vec(), btree_address: Some(btree_addr as u64), version: 3, chunk_index_type: None, @@ -2218,7 +2367,6 @@ mod tests { let os: u8 = 8; let elem_size = 8usize; - let ndims = 2; let chunk_elems = 10usize; let total = 20usize; @@ -2257,12 +2405,13 @@ mod tests { data_offset += compressed.len() + 16; // some padding } - let btree = build_chunk_btree_leaf(&chunk_infos, ndims, os); + let dims = [chunk_elems as u32, elem_size as u32]; + let btree = build_chunk_btree_leaf_dims(&chunk_infos, &dims, os); let btree_addr = 0x100usize; file_data[btree_addr..btree_addr + btree.len()].copy_from_slice(&btree); let layout = DataLayout::Chunked { - chunk_dimensions: vec![chunk_elems as u32, elem_size as u32], + chunk_dimensions: dims.to_vec(), btree_address: Some(btree_addr as u64), version: 3, chunk_index_type: None, @@ -2300,7 +2449,6 @@ mod tests { // 4x6 dataset with chunk size 2x3 => 4 chunks let os: u8 = 8; let elem_size = 4usize; // f32 - let ndims = 3; // rank(2) + 1 let ds_dims = [4usize, 6]; let chunk_dims = [2usize, 3]; @@ -2340,12 +2488,13 @@ mod tests { } } - let btree = build_chunk_btree_leaf(&chunk_infos, ndims, os); + let dims = [chunk_dims[0] as u32, chunk_dims[1] as u32, elem_size as u32]; + let btree = build_chunk_btree_leaf_dims(&chunk_infos, &dims, os); let btree_addr = 0x100usize; file_data[btree_addr..btree_addr + btree.len()].copy_from_slice(&btree); let layout = DataLayout::Chunked { - chunk_dimensions: vec![chunk_dims[0] as u32, chunk_dims[1] as u32, elem_size as u32], + chunk_dimensions: dims.to_vec(), btree_address: Some(btree_addr as u64), version: 3, chunk_index_type: None, diff --git a/crates/clawhdf5/tests/header_validation_interop.rs b/crates/clawhdf5/tests/header_validation_interop.rs index 25614c5..d0a1273 100644 --- a/crates/clawhdf5/tests/header_validation_interop.rs +++ b/crates/clawhdf5/tests/header_validation_interop.rs @@ -629,17 +629,13 @@ save("mdc_past_eof", bad) ); } -/// Chunk index entries HDF5 2.0 mis-reads, refused here. An unfiltered -/// chunk the index records at less than the chunk's size (`cve-2025-44904`): -/// HDF5 2.0 fills the rest of the chunk with whatever its buffer held, and -/// later libhdf5 releases refuse it ("incorrect chunk size returned from -/// index for unfiltered chunk"); we read the rest as zeros. A chunk keyed -/// with a non-zero element offset (`cve-2025-44905`): libhdf5's lookup -/// compares that coordinate too, so whether it finds the chunk depends on -/// where the key falls (in `cve-2025-44905` it does not, and h5py reads -/// fill values; in this file it does); we read the chunk. +/// An unfiltered chunk the index records at less than the chunk's size +/// (`cve-2025-44904`): HDF5 2.0 fills the rest of the chunk with whatever +/// its buffer held, and later libhdf5 releases refuse it ("incorrect chunk +/// size returned from index for unfiltered chunk"); we used to read the +/// rest as zeros, and now refuse it. #[test] -fn chunk_index_entries_libhdf5_misreads_are_refused() { +fn unfiltered_chunk_of_the_wrong_size_is_refused() { skip_if_no_python!(); let dir = tempfile::tempdir().unwrap(); run_python( @@ -658,8 +654,6 @@ second = key + 24 + 8 # a key (4 + 4 + 2 x 8 bytes), then a child address assert struct.unpack_from(" 0 and data[tree + 5] == 0 + ndims = len(shape) + 1 + key_size = 8 + 8 * ndims + key = tree + 8 + 16 + which * (key_size + 8) + last = key + 8 + 8 * (ndims - 1) + assert struct.unpack_from(" = std::fs::read_to_string(dir.path().join(format!("{name}.h5.txt"))) + .unwrap() + .split_whitespace() + .map(|v| v.parse().unwrap()) + .collect(); + let file = File::open(&path).unwrap(); + let got = file.dataset("d").unwrap().read_i32(); + assert_eq!(got.as_deref().ok(), Some(&expected[..]), "{name}: {got:?}"); } } From a6ed3a5c7d287e444358d307bc8dc545c6aca448 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:36:10 -0500 Subject: [PATCH 14/20] fix(format): resolve cache-image flush-dependency parents as libhdf5 does The review suggested libhdf5 loads every image entry and resolves flush-dependency parents afterwards. It does not: H5C__reconstruct_cache_contents (HDF5 1.14.6 and 2.0.0, and develop) inserts each entry and then searches the cache index for its parents in the same loop, failing with "fd parent not in cache?!?" when one is missing. So a parent must be an earlier image entry, as before, or metadata cached before the image loads: the superblock (address 0) and the superblock extension's object header, which libhdf5 reads to find the image. Those two were refused as parents; they are now accepted. A parent listed after its child is still refused, as libhdf5 refuses it, and so is an entry that is its own parent ("Child entry flush dependency parent can't be itself"). apply_cache_image takes the superblock to know the extension address. Test: superblock_ext::tests::flush_dependency_parents_must_already_be_cached (parent-first loads, child-first refused, extension header accepted, self-parent refused); the extension-header case fails without the fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- conformance/probe/src/main.rs | 2 +- crates/clawhdf5-format/src/superblock_ext.rs | 91 +++++++++++++++++--- 2 files changed, 81 insertions(+), 12 deletions(-) diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 9fa9929..fe619a0 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -731,7 +731,7 @@ fn main() { let view = match ext.and_then(|x| x.cache_image) { None => None, Some(loc) => match guarded(|| { - superblock_ext::apply_cache_image(hdf5, loc, sb.offset_size, sb.length_size) + superblock_ext::apply_cache_image(hdf5, loc, &sb) .map_err(e) }) { Ok(v) => Some(v), diff --git a/crates/clawhdf5-format/src/superblock_ext.rs b/crates/clawhdf5-format/src/superblock_ext.rs index d7ec3cc..2db3d8b 100644 --- a/crates/clawhdf5-format/src/superblock_ext.rs +++ b/crates/clawhdf5-format/src/superblock_ext.rs @@ -297,16 +297,16 @@ struct ImageEntry { /// (`H5C__decode_cache_image_header`, `H5C__reconstruct_cache_entry`): /// signature and version, the image length it records, entry types, rings /// and ages in range, entry addresses inside the file and not repeated, -/// flush-dependency parents that are earlier entries. +/// flush-dependency parents already in the cache. /// /// libhdf5 does not verify the block's trailing checksum when it loads an /// image, so neither does this. pub fn apply_cache_image( data: &[u8], location: CacheImageLocation, - offset_size: u8, - length_size: u8, + sb: &Superblock, ) -> Result, FormatError> { + let (offset_size, length_size) = (sb.offset_size, sb.length_size); let bad = FormatError::InvalidCacheImage; let start = usize::try_from(location.address).map_err(|_| bad("address out of range"))?; let len = usize::try_from(location.length).map_err(|_| bad("length out of range"))?; @@ -336,6 +336,17 @@ pub fn apply_cache_image( } let mut entries = Vec::new(); + // What is in libhdf5's cache when it loads the image: the superblock + // and the superblock extension's object header (read to find the image). + // Each entry's flush-dependency parents are looked up in the cache as + // the entry is inserted (`H5C__reconstruct_cache_contents` searches the + // index inside the loop that inserts the entries, in HDF5 1.14.6 and + // 2.0.0 alike), so a parent must be one of those or an earlier entry. + let mut cached = BTreeSet::new(); + cached.insert(0); + if let Some(ext) = sb.superblock_extension_address { + cached.insert(ext); + } let mut seen = BTreeSet::new(); for _ in 0..n_entries { let type_id = c.u8()?; @@ -374,8 +385,8 @@ pub fn apply_cache_image( let parent = c .addr(offset_size)? .ok_or(bad("invalid flush dependency parent offset"))?; - if !seen.contains(&parent) { - return Err(bad("flush dependency parent not in the image")); + if !seen.contains(&parent) && !cached.contains(&parent) { + return Err(bad("fd parent not in cache")); } } let len = usize::try_from(size).map_err(|_| bad(RAN_OFF))?; @@ -415,7 +426,7 @@ pub fn metadata_view(data: &[u8], sb: &Superblock) -> Result>, Fo Some(SuperblockExtension { cache_image: Some(location), .. - }) => apply_cache_image(data, location, sb.offset_size, sb.length_size).map(Some), + }) => apply_cache_image(data, location, sb).map(Some), _ => Ok(None), } } @@ -562,18 +573,37 @@ mod tests { /// A cache image block with `entries` of (address, bytes). fn image(entries: &[(u64, &[u8])]) -> Vec { + let with_deps: Vec<_> = entries.iter().map(|&(a, b)| (a, b, 0, None)).collect(); + image_with_deps(&with_deps) + } + + /// A cache image block with `entries` of (address, bytes, flush + /// dependency children, flush dependency parent). + fn image_with_deps(entries: &[(u64, &[u8], u16, Option)]) -> Vec { let mut b = Vec::new(); b.extend_from_slice(MDCI_SIGNATURE); b.push(0); b.push(0); b.extend_from_slice(&0u64.to_le_bytes()); // length, patched below b.extend_from_slice(&(entries.len() as u32).to_le_bytes()); - for (addr, bytes) in entries { - b.extend_from_slice(&[5, 0x02, 1, 0]); // type, flags (in LRU), ring, age - b.extend_from_slice(&[0; 6]); // children, dirty children, parents + for &(addr, bytes, children, parent) in entries { + let mut flags = 0x02; // in LRU + if children > 0 { + flags |= MDCI_ENTRY_IS_FD_PARENT; + } + if parent.is_some() { + flags |= MDCI_ENTRY_IS_FD_CHILD; + } + b.extend_from_slice(&[5, flags, 1, 0]); // type, flags, ring, age + b.extend_from_slice(&children.to_le_bytes()); + b.extend_from_slice(&0u16.to_le_bytes()); // dirty children + b.extend_from_slice(&u16::from(parent.is_some()).to_le_bytes()); b.extend_from_slice(&0i32.to_le_bytes()); b.extend_from_slice(&addr.to_le_bytes()); b.extend_from_slice(&(bytes.len() as u64).to_le_bytes()); + if let Some(p) = parent { + b.extend_from_slice(&p.to_le_bytes()); + } b.extend_from_slice(bytes); } b.extend_from_slice(&[0; 4]); // checksum (not verified, as in libhdf5) @@ -592,7 +622,7 @@ mod tests { address: at, length: img.len() as u64, }; - let out = apply_cache_image(&f, loc, 8, 8).unwrap(); + let out = apply_cache_image(&f, loc, &sb_v2(u64::MAX)).unwrap(); assert_eq!(out.len(), f.len()); assert_eq!(&out[16..22], b"HEADER"); assert_eq!(&out[40..44], b"NODE"); @@ -605,7 +635,7 @@ mod tests { address: 64, length: img.len() as u64, }; - apply_cache_image(&f, loc, 8, 8).unwrap_err() + apply_cache_image(&f, loc, &sb_v2(u64::MAX)).unwrap_err() }; let mut sig = image(&[(16, b"x")]); sig[0] = b'X'; @@ -627,4 +657,43 @@ mod tests { cut[6..14].copy_from_slice(&n.to_le_bytes()); assert!(matches!(bad(cut), FormatError::InvalidCacheImage(_))); } + + /// libhdf5 resolves an entry's flush-dependency parents as it inserts + /// the entry (`H5C__reconstruct_cache_contents`): a parent must be an + /// earlier entry, or the superblock or its extension's object header, + /// which are cached before the image loads. A parent listed after its + /// child fails ("fd parent not in cache?!?"). + #[test] + fn flush_dependency_parents_must_already_be_cached() { + let load = |img: Vec| { + let mut f = vec![0u8; 64]; + f.extend_from_slice(&img); + let loc = CacheImageLocation { + address: 64, + length: img.len() as u64, + }; + apply_cache_image(&f, loc, &sb_v2(48)) + }; + // Parent first, as libhdf5 writes images. + assert!( + load(image_with_deps(&[ + (16, b"P", 1, None), + (40, b"C", 0, Some(16)) + ])) + .is_ok() + ); + // Child first: libhdf5 does not find the parent. + assert_eq!( + load(image_with_deps(&[ + (40, b"C", 0, Some(16)), + (16, b"P", 1, None) + ])) + .unwrap_err(), + FormatError::InvalidCacheImage("fd parent not in cache") + ); + // The superblock extension's header (at 48 here) is in the cache. + assert!(load(image_with_deps(&[(40, b"C", 0, Some(48))])).is_ok()); + // An entry cannot be its own parent. + assert!(load(image_with_deps(&[(40, b"C", 1, Some(40))])).is_err()); + } } From 60502593b74b9ff62ccbc8376bc1aed7eaad24d3 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:42:43 -0500 Subject: [PATCH 15/20] fix: apply a metadata cache image without copying the file apply_cache_image returned a copy of the whole file with the image's entries written in, and File (mmap by default), MmapFile and LazyFile used that copy for every read: opening a 1 GiB sparse file with an image needed 2 GB of memory, and an 8 GiB one aborted the process, where de2a53f (which ignored the image) opened them in a few MB. The metadata parsers read one contiguous slice, so the image still has to be laid over the file's bytes; it is now laid over a private copy that costs only the pages it touches: - clawhdf5_format::superblock_ext::CacheImage decodes the image into an entry list (address, offset in the block, length) and applies it to any destination; cache_image_state tells an opener whether the file has no image, a loadable one, or one libhdf5 cannot load; apply_cache_image_in_place is for readers that own their buffer. apply_cache_image and metadata_view (which copied) are gone. - clawhdf5_io::HDF5Read::private_copy returns a writable private copy of a reader's bytes: MmapReader gives a MAP_PRIVATE copy-on-write mapping (memmap2 map_copy), so only the pages the entries land on are copied; the default copies the bytes (in-memory readers). - File, MmapFile and LazyFile write the image into that mapping (crate::cache_image). File::from_bytes / open_buffered patch their own buffer in place, copying only the image block, as libhdf5 does. A file without an image is read straight from the mapping, unchanged. An image entry that runs past the end of file is now refused: libhdf5 checks only that it starts inside the file, and the images libhdf5 writes never do this, but those bytes have nowhere to go in a view of the file. Tests: tests/cache_image_memory.rs has libhdf5 (through ctypes) add an image to a 1 GiB sparse file and bounds resident-memory growth for all three openers at 256 MiB; it fails on the previous commit (File::open grew 2,148,720,640 bytes). reader.rs zero_copy_tests check that a file without an image is read from the mapping itself and that an image goes into a copy-on-write mapping, not a heap copy; clawhdf5-io checks that private_copy writes never reach the reader or the file. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 16 +- conformance/probe/src/main.rs | 30 +- crates/clawhdf5-format/src/superblock_ext.rs | 369 ++++++++++++------- crates/clawhdf5-io/src/lib.rs | 59 +++ crates/clawhdf5-io/src/mmap.rs | 29 ++ crates/clawhdf5-io/src/prefetch.rs | 4 + crates/clawhdf5/src/cache_image.rs | 85 +++++ crates/clawhdf5/src/lazy.rs | 34 +- crates/clawhdf5/src/lib.rs | 1 + crates/clawhdf5/src/mmap_file.rs | 27 +- crates/clawhdf5/src/reader.rs | 78 +++- crates/clawhdf5/tests/cache_image_memory.rs | 139 +++++++ 12 files changed, 695 insertions(+), 176 deletions(-) create mode 100644 crates/clawhdf5/src/cache_image.rs create mode 100644 crates/clawhdf5/tests/cache_image_memory.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cc4248..3b0efd6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,19 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. bytes; in `h5clear_mdc_image.h5` the root group exists only there, and every reader failed with `InvalidObjectHeaderVersion(0)`. `File`, `MmapFile` and `LazyFile` (and `h5rs`) now apply the image at open - (`clawhdf5_format::superblock_ext`), with libhdf5's checks. A file whose - image libhdf5 cannot load opens in libhdf5 but nothing in it can be read; - `File::open` refuses it. + (`clawhdf5_format::superblock_ext::CacheImage`), with libhdf5's checks. + The file is not copied to do it: a mapped file gets the image's entries + written into a private copy-on-write mapping + (`clawhdf5_io::HDF5Read::private_copy`, `MAP_PRIVATE`), so only the pages + they land on are copied, and a buffer the opener owns (`File::from_bytes`, + `open_buffered`) is patched in place; files without an image are read + from the mapping exactly as before. (An interim version copied the whole + file onto the heap: 2 GB of memory to open a 1 GiB sparse file with an + image, and an abort for an 8 GiB one; `tests/cache_image_memory.rs` + guards it.) An image entry that runs past the end of file is refused + (libhdf5 checks only its start; the images it writes never do this). A + file whose image libhdf5 cannot load opens in libhdf5 but nothing in it + can be read; `File::open` refuses it. - **The superblock extension is decoded at open, as libhdf5 does:** a File Space Info or Metadata Cache Image message libhdf5 cannot decode makes the open fail (`cve-2020-10810`, `cve-2020-10812` were opened). diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index fe619a0..8c25a88 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -718,8 +718,8 @@ fn main() { // root group — so a file whose image it cannot load still opens and // every object fails; File::open refuses such a file outright. The // probe records the image's error where libhdf5 reports it. - use clawhdf5_format::superblock_ext; - let ext = match guarded(|| superblock_ext::read_superblock_extension(hdf5, &sb).map_err(e)) { + use clawhdf5_format::superblock_ext::{self, CacheImageState}; + let state = match guarded(|| superblock_ext::cache_image_state(hdf5, &sb).map_err(e)) { Ok(x) => x, Err(msg) => { top.insert("open_error".into(), Value::String(msg)); @@ -728,18 +728,22 @@ fn main() { } }; let mut image_error = None; - let view = match ext.and_then(|x| x.cache_image) { - None => None, - Some(loc) => match guarded(|| { - superblock_ext::apply_cache_image(hdf5, loc, &sb) - .map_err(e) - }) { - Ok(v) => Some(v), - Err(msg) => { - image_error = Some(msg); - None + let view = match state { + CacheImageState::Absent => None, + CacheImageState::Unloadable(err) => { + image_error = Some(e(err)); + None + } + CacheImageState::Loaded(image) => { + let mut v = hdf5.to_vec(); + match image.block(hdf5).and_then(|b| image.apply(b, &mut v)) { + Ok(()) => Some(v), + Err(err) => { + image_error = Some(e(err)); + None + } } - }, + } }; let hdf5: &[u8] = view.as_deref().unwrap_or(hdf5); top.insert("superblock_version".into(), json!(sb.version)); diff --git a/crates/clawhdf5-format/src/superblock_ext.rs b/crates/clawhdf5-format/src/superblock_ext.rs index 2db3d8b..e1d1cf3 100644 --- a/crates/clawhdf5-format/src/superblock_ext.rs +++ b/crates/clawhdf5-format/src/superblock_ext.rs @@ -14,9 +14,11 @@ //! `H5C__reconstruct_cache_contents`), and the entries take the place of //! the file's bytes at their addresses: the file itself may hold stale or //! no metadata there (in `h5clear_mdc_image.h5` the root group's header is -//! only in the image). [`apply_cache_image`] does the same with bytes: it -//! returns a copy of the file with every entry written at its address, so -//! every parser reads what libhdf5 reads. +//! only in the image). [`CacheImage::apply`] does the same with bytes: it +//! writes every entry at its address, so every parser reads what libhdf5 +//! reads. It writes into whatever the opener gives it — a private +//! copy-on-write mapping of the file, or a buffer the opener owns — so the +//! file is never copied whole. #[cfg(not(feature = "std"))] use alloc::{collections::BTreeSet, vec::Vec}; @@ -284,150 +286,243 @@ fn decode_fsinfo(data: &[u8], os: u8, ls: u8) -> Result Result, FormatError> { - let (offset_size, length_size) = (sb.offset_size, sb.length_size); + entries: Vec, +} + +/// What an opener must do about a file's metadata cache image. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CacheImageState { + /// The file has no image: its bytes are its metadata. + Absent, + /// The file has an image that loads: apply it with [`CacheImage::apply`]. + Loaded(CacheImage), + /// The file has an image libhdf5 fails to load. libhdf5 still opens the + /// file (the image loads at the first metadata read), and that read + /// fails with this error. + Unloadable(FormatError), +} + +impl CacheImage { + /// Decode the metadata cache image at `location` in `data` (the file + /// from the superblock on, up to its recorded end of file). The image is + /// checked as libhdf5 checks it (`H5C__decode_cache_image_header`, + /// `H5C__reconstruct_cache_entry`): signature and version, the image + /// length it records, entry types, rings and ages in range, entry + /// addresses inside the file and not repeated, flush-dependency parents + /// already in the cache. + /// + /// One check is stricter than libhdf5's: an entry must end inside the + /// file. libhdf5 checks only that it starts there, and serves the rest + /// from the image; the images libhdf5 writes never do this (every entry + /// lies below the image block, which is written last), and the bytes an + /// entry would put past the end of file have nowhere to go in a view of + /// the file. + /// + /// libhdf5 does not verify the block's trailing checksum when it loads + /// an image, so neither does this. + pub fn decode( + data: &[u8], + location: CacheImageLocation, + sb: &Superblock, + ) -> Result { + let (offset_size, length_size) = (sb.offset_size, sb.length_size); + let bad = FormatError::InvalidCacheImage; + let block = image_block(data, location)?; + let eoa = data.len() as u64; + let mut c = Cursor::new(block, bad(RAN_OFF)); + + // Header: signature, version, flags, image data length, entry count. + if c.take(4)? != MDCI_SIGNATURE { + return Err(bad("bad metadata cache image header signature")); + } + if c.u8()? != 0 { + return Err(bad("bad metadata cache image version")); + } + if c.u8()? & MDCI_HAVE_RESIZE_STATUS != 0 { + return Err(bad("MDC resize status not yet supported")); + } + if c.uint(length_size)? != location.length { + return Err(bad("bad metadata cache image data length")); + } + let n_entries = c.uint(4)?; + if n_entries == 0 { + return Err(bad("bad metadata cache entry count")); + } + + let mut entries = Vec::new(); + // What is in libhdf5's cache when it loads the image: the superblock + // and the superblock extension's object header (read to find the + // image). Each entry's flush-dependency parents are looked up in the + // cache as the entry is inserted (`H5C__reconstruct_cache_contents` + // searches the index inside the loop that inserts the entries, in + // HDF5 1.14.6 and 2.0.0 alike), so a parent must be one of those or + // an earlier entry. + let mut cached = BTreeSet::new(); + cached.insert(0); + if let Some(ext) = sb.superblock_extension_address { + cached.insert(ext); + } + let mut seen = BTreeSet::new(); + for _ in 0..n_entries { + let type_id = c.u8()?; + if type_id >= MDCI_NTYPES { + return Err(bad("type id is out of valid range")); + } + let flags = c.u8()?; + if c.u8()? >= MDCI_RING_NTYPES { + return Err(bad("ring is out of valid range")); + } + if c.u8()? > MDCI_AGE_MAX { + return Err(bad("entry age is out of policy range")); + } + let children = c.uint(2)?; + // libhdf5 checks the parent flag against the child count only in + // debug builds (release builds refuse any entry with children); + // the image format's own rule is checked here. + if (flags & MDCI_ENTRY_IS_FD_PARENT != 0) != (children > 0) { + return Err(bad("flush dependency parent flag and child count disagree")); + } + c.uint(2)?; // dirty dependency children: reset for a read-only open + let parents = c.uint(2)?; + if (flags & MDCI_ENTRY_IS_FD_CHILD != 0) != (parents > 0) { + return Err(bad("flush dependency child flag and parent count disagree")); + } + c.uint(4)?; // LRU rank + let address = c + .addr(offset_size)? + .filter(|&a| a < eoa) + .ok_or(bad("invalid entry address range"))?; + let size = c.uint(length_size)?; + if size == 0 { + return Err(bad("invalid entry size")); + } + for _ in 0..parents { + let parent = c + .addr(offset_size)? + .ok_or(bad("invalid flush dependency parent offset"))?; + if !seen.contains(&parent) && !cached.contains(&parent) { + return Err(bad("fd parent not in cache")); + } + } + let len = usize::try_from(size).map_err(|_| bad(RAN_OFF))?; + let image_offset = c.pos; + c.take(len)?; + if address.checked_add(size).is_none_or(|end| end > eoa) { + return Err(bad("entry extends past the end of file")); + } + if !seen.insert(address) { + return Err(bad("duplicate addresses in cache")); + } + entries.push(ImageEntry { + address, + image_offset, + len, + }); + } + Ok(CacheImage { location, entries }) + } + + /// Where the image block is. + pub fn location(&self) -> CacheImageLocation { + self.location + } + + /// The number of entries in the image. + pub fn len(&self) -> usize { + self.entries.len() + } + + /// Whether the image has no entries (a decoded image always has some). + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + + /// The file ranges (address, length) the image's entries replace. + pub fn entry_ranges(&self) -> impl Iterator + '_ { + self.entries.iter().map(|e| (e.address, e.len)) + } + + /// The image block in `data`, the bytes [`Self::decode`] read it from. + pub fn block<'a>(&self, data: &'a [u8]) -> Result<&'a [u8], FormatError> { + image_block(data, self.location) + } + + /// Write every entry over `dst`, the file's bytes from the superblock + /// on (as long as the `data` the image was decoded from), taking the + /// entries from `block` (the image block, see [`Self::block`]). `block` + /// must not alias `dst`: an entry may land on the block itself. + pub fn apply(&self, block: &[u8], dst: &mut [u8]) -> Result<(), FormatError> { + let short = || FormatError::InvalidCacheImage("image applied to the wrong file"); + for e in &self.entries { + let src = block + .get(e.image_offset..e.image_offset + e.len) + .ok_or_else(short)?; + let at = usize::try_from(e.address).map_err(|_| short())?; + dst.get_mut(at..at + e.len) + .ok_or_else(short)? + .copy_from_slice(src); + } + Ok(()) + } +} + +fn image_block(data: &[u8], location: CacheImageLocation) -> Result<&[u8], FormatError> { let bad = FormatError::InvalidCacheImage; let start = usize::try_from(location.address).map_err(|_| bad("address out of range"))?; let len = usize::try_from(location.length).map_err(|_| bad("length out of range"))?; - let block = start + start .checked_add(len) .and_then(|end| data.get(start..end)) - .ok_or(bad("image block extends past the end of the file"))?; - let eoa = data.len() as u64; - let mut c = Cursor::new(block, bad(RAN_OFF)); - - // Header: signature, version, flags, image data length, entry count. - if c.take(4)? != MDCI_SIGNATURE { - return Err(bad("bad metadata cache image header signature")); - } - if c.u8()? != 0 { - return Err(bad("bad metadata cache image version")); - } - if c.u8()? & MDCI_HAVE_RESIZE_STATUS != 0 { - return Err(bad("MDC resize status not yet supported")); - } - if c.uint(length_size)? != location.length { - return Err(bad("bad metadata cache image data length")); - } - let n_entries = c.uint(4)?; - if n_entries == 0 { - return Err(bad("bad metadata cache entry count")); - } - - let mut entries = Vec::new(); - // What is in libhdf5's cache when it loads the image: the superblock - // and the superblock extension's object header (read to find the image). - // Each entry's flush-dependency parents are looked up in the cache as - // the entry is inserted (`H5C__reconstruct_cache_contents` searches the - // index inside the loop that inserts the entries, in HDF5 1.14.6 and - // 2.0.0 alike), so a parent must be one of those or an earlier entry. - let mut cached = BTreeSet::new(); - cached.insert(0); - if let Some(ext) = sb.superblock_extension_address { - cached.insert(ext); - } - let mut seen = BTreeSet::new(); - for _ in 0..n_entries { - let type_id = c.u8()?; - if type_id >= MDCI_NTYPES { - return Err(bad("type id is out of valid range")); - } - let flags = c.u8()?; - if c.u8()? >= MDCI_RING_NTYPES { - return Err(bad("ring is out of valid range")); - } - if c.u8()? > MDCI_AGE_MAX { - return Err(bad("entry age is out of policy range")); - } - let children = c.uint(2)?; - // libhdf5 checks the parent flag against the child count only in - // debug builds (release builds refuse any entry with children); the - // image format's own rule is checked here. - if (flags & MDCI_ENTRY_IS_FD_PARENT != 0) != (children > 0) { - return Err(bad("flush dependency parent flag and child count disagree")); - } - c.uint(2)?; // dirty dependency children: reset for a read-only open - let parents = c.uint(2)?; - if (flags & MDCI_ENTRY_IS_FD_CHILD != 0) != (parents > 0) { - return Err(bad("flush dependency child flag and parent count disagree")); - } - c.uint(4)?; // LRU rank - let address = c - .addr(offset_size)? - .filter(|&a| a < eoa) - .ok_or(bad("invalid entry address range"))?; - let size = c.uint(length_size)?; - if size == 0 { - return Err(bad("invalid entry size")); - } - for _ in 0..parents { - let parent = c - .addr(offset_size)? - .ok_or(bad("invalid flush dependency parent offset"))?; - if !seen.contains(&parent) && !cached.contains(&parent) { - return Err(bad("fd parent not in cache")); - } - } - let len = usize::try_from(size).map_err(|_| bad(RAN_OFF))?; - let image_offset = c.pos; - c.take(len)?; - if !seen.insert(address) { - return Err(bad("duplicate addresses in cache")); - } - entries.push(ImageEntry { - address, - image_offset, - len, - }); - } - - let mut out = data.to_vec(); - for e in &entries { - // address < eoa <= usize::MAX, and the entry's bytes came from the - // block, so neither conversion nor the sum can fail. - let at = e.address as usize; - let end = at + e.len; - if end > out.len() { - out.resize(end, 0); - } - out[at..end].copy_from_slice(&block[e.image_offset..e.image_offset + e.len]); - } - Ok(out) + .ok_or(bad("image block extends past the end of the file")) } -/// What a reader must do before reading a file's metadata, in one call: -/// check the superblock extension ([`read_superblock_extension`]) and, -/// when the file has a metadata cache image, return the file's bytes with -/// the image applied ([`apply_cache_image`]). `Ok(None)` means read `data` -/// as it is. -pub fn metadata_view(data: &[u8], sb: &Superblock) -> Result>, FormatError> { +/// What an opener must do before reading a file's metadata: check the +/// superblock extension ([`read_superblock_extension`]; an error means +/// libhdf5 refuses to open the file) and decode any metadata cache image +/// ([`CacheImage::decode`]). `data` is the file from the superblock on, up +/// to its recorded end of file. +pub fn cache_image_state(data: &[u8], sb: &Superblock) -> Result { match read_superblock_extension(data, sb)? { Some(SuperblockExtension { cache_image: Some(location), .. - }) => apply_cache_image(data, location, sb).map(Some), - _ => Ok(None), + }) => Ok(match CacheImage::decode(data, location, sb) { + Ok(image) => CacheImageState::Loaded(image), + Err(e) => CacheImageState::Unloadable(e), + }), + _ => Ok(CacheImageState::Absent), + } +} + +/// [`cache_image_state`] for a reader that holds the file's bytes in a +/// buffer of its own: check the superblock extension and write any cache +/// image over `data` in place (only the image block is copied). An image +/// libhdf5 cannot load is an error here: such a reader has no way to open +/// the file and fail each object instead. +pub fn apply_cache_image_in_place(data: &mut [u8], sb: &Superblock) -> Result<(), FormatError> { + match cache_image_state(data, sb)? { + CacheImageState::Absent => Ok(()), + CacheImageState::Unloadable(e) => Err(e), + CacheImageState::Loaded(image) => { + let block = image.block(data)?.to_vec(); + image.apply(&block, data) + } } } @@ -435,6 +530,18 @@ pub fn metadata_view(data: &[u8], sb: &Superblock) -> Result>, Fo mod tests { use super::*; + /// The file's bytes with the image at `loc` applied. + fn apply_cache_image( + data: &[u8], + loc: CacheImageLocation, + sb: &Superblock, + ) -> Result, FormatError> { + let image = CacheImage::decode(data, loc, sb)?; + let mut out = data.to_vec(); + image.apply(image.block(data)?, &mut out)?; + Ok(out) + } + fn sb_v2(ext: u64) -> Superblock { Superblock { version: 2, @@ -651,6 +758,12 @@ mod tests { let mut len = image(&[(16, b"x")]); len[6] ^= 1; assert!(matches!(bad(len), FormatError::InvalidCacheImage(_))); + // An entry that starts inside the file (64 bytes, then a 60-byte + // image) but runs past its end. + assert!(matches!( + bad(image(&[(123, b"8 bytes!")])), + FormatError::InvalidCacheImage("entry extends past the end of file") + )); let mut cut = image(&[(16, b"abcdef")]); let n = cut.len() as u64 - 8; cut.truncate(cut.len() - 8); diff --git a/crates/clawhdf5-io/src/lib.rs b/crates/clawhdf5-io/src/lib.rs index 1f42019..5de13b7 100644 --- a/crates/clawhdf5-io/src/lib.rs +++ b/crates/clawhdf5-io/src/lib.rs @@ -41,6 +41,65 @@ pub trait HDF5Read { fn is_empty(&self) -> bool { self.as_bytes().is_empty() } + + /// A private, writable copy of [`Self::as_bytes`]: writes to it stay in + /// this process and never reach the underlying storage. + /// + /// Readers use it to lay a file's metadata cache image over the file's + /// own metadata. The default copies the bytes; a memory-mapped reader + /// returns a copy-on-write mapping instead, so only the pages written to + /// are copied and the rest stay shared with the page cache. + fn private_copy(&self) -> io::Result { + Ok(PrivateCopy::Owned(self.as_bytes().to_vec())) + } +} + +/// A private, writable copy of a file's bytes (see +/// [`HDF5Read::private_copy`]). +pub enum PrivateCopy { + /// The bytes copied onto the heap. + Owned(Vec), + /// A copy-on-write mapping of the file: pages are copied only when + /// written to. + #[cfg(feature = "mmap")] + Mapped(memmap2::MmapMut), +} + +impl PrivateCopy { + /// Whether this is a copy-on-write mapping rather than a heap copy. + pub fn is_mapped(&self) -> bool { + !matches!(self, PrivateCopy::Owned(_)) + } +} + +impl std::fmt::Debug for PrivateCopy { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("PrivateCopy") + .field("len", &self.len()) + .field("mapped", &self.is_mapped()) + .finish() + } +} + +impl std::ops::Deref for PrivateCopy { + type Target = [u8]; + fn deref(&self) -> &[u8] { + match self { + PrivateCopy::Owned(v) => v, + #[cfg(feature = "mmap")] + PrivateCopy::Mapped(m) => m, + } + } +} + +impl std::ops::DerefMut for PrivateCopy { + fn deref_mut(&mut self) -> &mut [u8] { + match self { + PrivateCopy::Owned(v) => v, + #[cfg(feature = "mmap")] + PrivateCopy::Mapped(m) => m, + } + } } /// Read-write access to HDF5 data. diff --git a/crates/clawhdf5-io/src/mmap.rs b/crates/clawhdf5-io/src/mmap.rs index 70d99c9..756d5e3 100644 --- a/crates/clawhdf5-io/src/mmap.rs +++ b/crates/clawhdf5-io/src/mmap.rs @@ -87,6 +87,19 @@ impl HDF5Read for MmapReader { fn as_bytes(&self) -> &[u8] { &self.mmap } + + /// A private copy-on-write mapping of the file (`MAP_PRIVATE`): only the + /// pages written to are copied. + fn private_copy(&self) -> io::Result { + if self.mmap.is_empty() { + return Ok(crate::PrivateCopy::Owned(Vec::new())); + } + // SAFETY: as for `open`: the caller keeps the file from being + // modified while the mapping is alive. Writes to a private mapping + // never reach the file. + let map = unsafe { memmap2::MmapOptions::new().map_copy(&self._file)? }; + Ok(crate::PrivateCopy::Mapped(map)) + } } /// Writable memory-mapped file for read-write HDF5 access. @@ -218,6 +231,22 @@ mod tests { fs::remove_file(&path).ok(); } + #[test] + fn private_copy_is_a_copy_on_write_mapping() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cow.bin"); + fs::write(&path, [1u8, 2, 3, 4]).unwrap(); + let reader = MmapReader::open(&path).unwrap(); + let mut copy = reader.private_copy().unwrap(); + assert!(copy.is_mapped()); + copy[1] = 99; + assert_eq!(©[..], &[1, 99, 3, 4]); + // Neither the reader's mapping nor the file sees the write. + assert_eq!(reader.as_bytes(), &[1, 2, 3, 4]); + drop(copy); + assert_eq!(fs::read(&path).unwrap(), [1, 2, 3, 4]); + } + #[test] fn mmap_reader_read_at() { let dir = std::env::temp_dir(); diff --git a/crates/clawhdf5-io/src/prefetch.rs b/crates/clawhdf5-io/src/prefetch.rs index a4dc451..bf3ad9b 100644 --- a/crates/clawhdf5-io/src/prefetch.rs +++ b/crates/clawhdf5-io/src/prefetch.rs @@ -195,6 +195,10 @@ impl HDF5Read for PrefetchReader { fn as_bytes(&self) -> &[u8] { self.inner.as_bytes() } + + fn private_copy(&self) -> std::io::Result { + self.inner.private_copy() + } } // --------------------------------------------------------------------------- diff --git a/crates/clawhdf5/src/cache_image.rs b/crates/clawhdf5/src/cache_image.rs new file mode 100644 index 0000000..1ceb741 --- /dev/null +++ b/crates/clawhdf5/src/cache_image.rs @@ -0,0 +1,85 @@ +//! Metadata cache images, as the file openers apply them. +//! +//! A file written with a metadata cache image keeps metadata cache entries +//! (object headers, B-tree nodes, heaps) in an image block, and libhdf5 +//! reads those entries in place of the file's own bytes at their addresses +//! (see `clawhdf5_format::superblock_ext`). The metadata parsers read one +//! contiguous byte slice, so the image has to be laid over the file's bytes +//! — without copying the file: +//! +//! - an opener that holds the file in a buffer it owns writes the entries +//! into that buffer (only the image block is copied, as libhdf5 copies +//! it); +//! - an opener that maps the file ([`File::open`](crate::File::open), +//! [`MmapFile`](crate::MmapFile), [`LazyFile::open_mmap`] +//! (crate::LazyFile::open_mmap)) writes them into a private copy-on-write +//! mapping of the file ([`clawhdf5_io::HDF5Read::private_copy`]): only the +//! pages the entries land on are copied, and the rest of the file stays +//! shared with the page cache; +//! - a file without an image is read from the original bytes, as before. + +use clawhdf5_format::error::FormatError; +use clawhdf5_format::superblock::Superblock; +use clawhdf5_format::superblock_ext::{self, CacheImageState}; +use clawhdf5_io::PrivateCopy; + +use crate::error::Error; + +/// A file's metadata, as an opener must read it. +pub(crate) enum ImageView { + /// Read the opener's bytes: the file has no cache image, or it was + /// written into a buffer the opener owns. + Plain, + /// The file with its cache image written in: a private copy of the + /// whole file (the HDF5 data at the same offsets as in the file). + Patched(PrivateCopy), + /// The file has a cache image libhdf5 cannot load. + Unloadable(FormatError), +} + +/// Check the superblock extension of the file whose bytes are `whole` (the +/// HDF5 data in `base..end`) and lay any cache image over a private copy +/// of the file made by `copy`. An error means libhdf5 refuses the file. +pub(crate) fn private_view( + whole: &[u8], + base: usize, + end: usize, + sb: &Superblock, + copy: impl FnOnce() -> std::io::Result, +) -> Result { + let data = &whole[base..end]; + Ok(match superblock_ext::cache_image_state(data, sb)? { + CacheImageState::Absent => ImageView::Plain, + CacheImageState::Unloadable(e) => ImageView::Unloadable(e), + CacheImageState::Loaded(image) => { + let mut view = copy().map_err(Error::Io)?; + let dst = + view.get_mut(base..end) + .ok_or(Error::Format(FormatError::InvalidCacheImage( + "the file changed while it was opened", + )))?; + image.apply(image.block(data)?, dst)?; + ImageView::Patched(view) + } + }) +} + +/// [`private_view`] for a file held in `whole`, a buffer the opener owns: +/// the image is written into it in place. +pub(crate) fn in_place( + whole: &mut [u8], + base: usize, + end: usize, + sb: &Superblock, +) -> Result { + let data = &mut whole[base..end]; + Ok(match superblock_ext::cache_image_state(data, sb)? { + CacheImageState::Absent => ImageView::Plain, + CacheImageState::Unloadable(e) => ImageView::Unloadable(e), + CacheImageState::Loaded(image) => { + let block = image.block(data)?.to_vec(); + image.apply(&block, data)?; + ImageView::Plain + } + }) +} diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index 8746f5f..cb688e9 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -46,9 +46,13 @@ pub struct LazyFile { /// End of the HDF5 data (`Superblock::data_end`, absolute). end: usize, superblock: Superblock, - /// The metadata as libhdf5 reads it when the file holds a metadata - /// cache image (see `superblock_ext::metadata_view`); `None` otherwise. - overlay: Option>, + /// A file that holds a metadata cache image, with the image written in: + /// the reader's [`HDF5Read::private_copy`] of the whole file (a + /// copy-on-write mapping for [`clawhdf5_io::MmapReader`], so only the + /// pages the image's entries land on are copied; see + /// `crate::cache_image`). `None` for a file without an image, read + /// straight from the reader. + patched: Option, root_header: ObjectHeader, /// Cache of parsed object headers, keyed by address. header_cache: RefCell>, @@ -87,11 +91,19 @@ impl LazyFile { let end = base + superblock.data_end(base as u64, whole_len)? as usize; // Decode the superblock extension as libhdf5 does at open, and load // a metadata cache image over the file's metadata. - let overlay = clawhdf5_format::superblock_ext::metadata_view( - &reader.as_bytes()[base..end], - &superblock, - )?; - let data = overlay.as_deref().unwrap_or(&reader.as_bytes()[base..end]); + let view = + crate::cache_image::private_view(reader.as_bytes(), base, end, &superblock, || { + reader.private_copy() + })?; + let patched = match view { + crate::cache_image::ImageView::Plain => None, + crate::cache_image::ImageView::Patched(p) => Some(p), + crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), + }; + let data = match &patched { + Some(p) => &p[base..end], + None => &reader.as_bytes()[base..end], + }; let root_header = ObjectHeader::parse( data, superblock.root_group_address as usize, @@ -103,7 +115,7 @@ impl LazyFile { base, end, superblock, - overlay, + patched, root_header, header_cache: RefCell::new(HashMap::new()), }) @@ -121,8 +133,8 @@ impl LazyFile { } fn hdf5_bytes(&self) -> &[u8] { - match &self.overlay { - Some(v) => v, + match &self.patched { + Some(p) => &p[self.base..self.end], None => &self.reader.as_bytes()[self.base..self.end], } } diff --git a/crates/clawhdf5/src/lib.rs b/crates/clawhdf5/src/lib.rs index f8098fa..222e432 100644 --- a/crates/clawhdf5/src/lib.rs +++ b/crates/clawhdf5/src/lib.rs @@ -24,6 +24,7 @@ //! builder.write("output.h5").unwrap(); //! ``` +mod cache_image; pub mod error; pub mod lazy; #[cfg(feature = "mmap")] diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 05d581a..0de5b02 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -38,9 +38,11 @@ pub struct MmapFile { /// End of the HDF5 data (`Superblock::data_end`, absolute). end: usize, superblock: Superblock, - /// The metadata as libhdf5 reads it when the file holds a metadata - /// cache image (see `superblock_ext::metadata_view`); `None` otherwise. - overlay: Option>, + /// A file that holds a metadata cache image, with the image written in: + /// a private copy-on-write mapping of the whole file, so only the pages + /// the image's entries land on are copied (see `crate::cache_image`). + /// `None` for a file without an image, read straight from the mapping. + patched: Option, } impl MmapFile { @@ -55,24 +57,29 @@ impl MmapFile { let end = base + superblock.data_end(base as u64, whole_len)? as usize; // Decode the superblock extension as libhdf5 does at open, and load // a metadata cache image over the file's metadata. - let overlay = clawhdf5_format::superblock_ext::metadata_view( - &reader.as_bytes()[base..end], - &superblock, - )?; + let view = + crate::cache_image::private_view(reader.as_bytes(), base, end, &superblock, || { + clawhdf5_io::HDF5Read::private_copy(&reader) + })?; + let patched = match view { + crate::cache_image::ImageView::Plain => None, + crate::cache_image::ImageView::Patched(p) => Some(p), + crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), + }; Ok(Self { reader, base, end, superblock, - overlay, + patched, }) } /// The file's bytes from the superblock on — the space HDF5 addresses /// index into. fn hdf5_bytes(&self) -> &[u8] { - match &self.overlay { - Some(v) => v, + match &self.patched { + Some(p) => &p[self.base..self.end], None => &self.reader.as_bytes()[self.base..self.end], } } diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index c892eea..9f6da0f 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -20,8 +20,8 @@ use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_format::signature; use clawhdf5_format::superblock::Superblock; -use clawhdf5_format::superblock_ext; +use crate::cache_image::{self, ImageView}; use crate::error::Error; use crate::types::{AttrValue, DType, classify_datatype, read_attrs}; @@ -56,17 +56,19 @@ struct FileData { base: usize, /// End of the HDF5 data in the file (`Superblock::data_end`, absolute). end: usize, - /// The file's metadata as libhdf5 reads it when the file holds a - /// metadata cache image: the bytes from the superblock to the end of - /// file with the image's entries written in - /// ([`superblock_ext::metadata_view`]). `None` for every other file. - overlay: Option>, + /// A mapped file that holds a metadata cache image, with the image + /// written in: a private copy-on-write mapping of the whole file, so + /// only the pages the image's entries land on are copied (see + /// `crate::cache_image`). `None` for every other file: a file without + /// an image is read straight from the mapping, and an owned buffer has + /// the image written into it in place. + patched: Option, } impl FileData { /// Locate the superblock and parse it. A truncated file is refused, and /// bytes past the recorded end of file are not read, as in libhdf5. - fn new(backing: Backing) -> Result<(Self, Superblock), Error> { + fn new(mut backing: Backing) -> Result<(Self, Superblock), Error> { let whole = backing.whole_file(); let (user_block, hdf5) = signature::split_user_block(whole)?; let base = user_block.len(); @@ -77,21 +79,34 @@ impl FileData { // libhdf5 decodes the superblock extension at open (a message it // cannot decode fails the open) and loads a metadata cache image // over the file's own metadata. - let overlay = superblock_ext::metadata_view(&whole[base..end], &superblock)?; + let view = match &mut backing { + Backing::Owned(v) => cache_image::in_place(v, base, end, &superblock)?, + #[cfg(feature = "mmap")] + Backing::Mmap(r) => { + cache_image::private_view(r.as_bytes(), base, end, &superblock, || { + clawhdf5_io::HDF5Read::private_copy(r) + })? + } + }; + let patched = match view { + ImageView::Plain => None, + ImageView::Patched(p) => Some(p), + ImageView::Unloadable(e) => return Err(e.into()), + }; Ok(( Self { backing, base, end, - overlay, + patched, }, superblock, )) } fn as_bytes(&self) -> &[u8] { - match &self.overlay { - Some(v) => v, + match &self.patched { + Some(p) => &p[self.base..self.end], None => &self.backing.whole_file()[self.base..self.end], } } @@ -1282,3 +1297,44 @@ mod sibling_file_name_tests { } } } + +#[cfg(all(test, feature = "mmap"))] +mod zero_copy_tests { + use super::*; + + /// Where `File::open` reads metadata from: `Some(true)` for the file's + /// own mapping, `Some(false)` for a private copy-on-write mapping. + fn reads_from_the_mapping(f: &File) -> Option { + let Backing::Mmap(r) = &f.data.backing else { + return None; + }; + let mapped = r.as_bytes()[f.data.base..].as_ptr(); + match &f.data.patched { + None => Some(std::ptr::eq(f.as_bytes().as_ptr(), mapped)), + Some(p) => { + assert!(p.is_mapped(), "the image went into a heap copy of the file"); + Some(false) + } + } + } + + #[test] + fn a_file_without_a_cache_image_is_read_from_the_mapping() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("plain.h5"); + let mut b = crate::FileBuilder::new(); + b.create_dataset("d").with_f64_data(&[1.0, 2.0]); + b.write(&path).unwrap(); + let f = File::open(&path).unwrap(); + assert_eq!(reads_from_the_mapping(&f), Some(true)); + assert_eq!(f.dataset("d").unwrap().read_f64().unwrap(), [1.0, 2.0]); + } + + #[test] + fn a_cache_image_goes_into_a_copy_on_write_mapping() { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/h5clear_mdc_image.h5"); + let f = File::open(path).unwrap(); + assert_eq!(reads_from_the_mapping(&f), Some(false)); + } +} diff --git a/crates/clawhdf5/tests/cache_image_memory.rs b/crates/clawhdf5/tests/cache_image_memory.rs new file mode 100644 index 0000000..16186ba --- /dev/null +++ b/crates/clawhdf5/tests/cache_image_memory.rs @@ -0,0 +1,139 @@ +//! Opening a file with a metadata cache image must not copy the file. +//! +//! The image's entries are laid over the file's bytes in a private +//! copy-on-write mapping (`File::open`, `MmapFile::open`, +//! `LazyFile::open_mmap`), so only the pages they land on are copied. The +//! first implementation copied the whole file onto the heap at open: a +//! 1 GiB file that takes a few KB on disk needed 2 GB of memory, and an +//! 8 GiB one aborted the process. Here libhdf5 itself (the library h5py +//! bundles, through ctypes: `H5Pset_mdc_image_config`) adds an image to a +//! 1 GiB sparse file, and the process's resident memory must stay far below +//! the file's size while each opener lists the file and reads its small +//! dataset. +//! +//! One test in its own binary, so no other test's allocations land in the +//! measurement. Linux only (it reads `VmRSS` from `/proc/self/status`). +//! Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +#![cfg(target_os = "linux")] + +use std::path::Path; +use std::process::Command; + +use clawhdf5::{File, LazyFile, MmapFile}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +fn rss_bytes() -> u64 { + let status = std::fs::read_to_string("/proc/self/status").unwrap(); + let line = status.lines().find(|l| l.starts_with("VmRSS:")).unwrap(); + let kb: u64 = line.split_whitespace().nth(1).unwrap().parse().unwrap(); + kb * 1024 +} + +/// A 1 GiB sparse file: `/big`, 2^27 `f8` with only its last element +/// written, `/small` = 0..10, with a metadata cache image added by libhdf5. +fn make_file(path: &Path) { + let script = format!( + r#" +import ctypes, glob, os, h5py, numpy as np +path = "{path}" +libs = glob.glob(os.path.join(os.path.dirname(h5py.__file__), os.pardir, "h5py.libs", "libhdf5-*.so*")) +assert libs, "no libhdf5 bundled with h5py" +lib = ctypes.CDLL(libs[0]) +class Cfg(ctypes.Structure): + _fields_ = [("version", ctypes.c_int), ("generate_image", ctypes.c_bool), + ("save_resize_status", ctypes.c_bool), ("entry_ageout", ctypes.c_int)] +with h5py.File(path, "w", libver="latest") as f: + d = f.create_dataset("big", shape=(2**27,), dtype="f8") + d[-1] = 7.5 + f.create_dataset("small", data=np.arange(10, dtype="= 0 +f = h5py.File(h5py.h5f.open(path.encode(), h5py.h5f.ACC_RDWR, fapl=fapl)) +f["small"][()]; f["big"].shape +f.close() +assert os.path.getsize(path) >= 2**30 +with open(path, "rb") as fh: + fh.seek(-(1 << 20), 2) + assert b"MDCI" in fh.read(), "libhdf5 wrote no cache image" +with h5py.File(path, "r") as f: + assert list(f["small"][()]) == list(range(10)) +"#, + path = path.display() + ); + let out = Command::new(python()) + .args(["-c", &script]) + .output() + .expect("failed to run python"); + assert!( + out.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); +} + +#[test] +fn a_cache_image_does_not_copy_the_file() { + if !python_available() { + assert!( + !std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1"), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("sparse_image.h5"); + make_file(&path); + + const LIMIT: u64 = 256 << 20; + let small: Vec = (0..10).collect(); + let before = rss_bytes(); + { + let f = File::open(&path).unwrap(); + let mut names = f.root().datasets().unwrap(); + names.sort(); + assert_eq!(names, ["big", "small"]); + assert_eq!(f.dataset("small").unwrap().read_i32().unwrap(), small); + assert_eq!(f.dataset("big").unwrap().shape().unwrap(), [1 << 27]); + let grew = rss_bytes().saturating_sub(before); + assert!( + grew < LIMIT, + "File::open: resident memory grew {grew} bytes" + ); + } + let before = rss_bytes(); + { + let f = MmapFile::open(&path).unwrap(); + assert_eq!(f.dataset("small").unwrap().read_i32().unwrap(), small); + let grew = rss_bytes().saturating_sub(before); + assert!( + grew < LIMIT, + "MmapFile::open: resident memory grew {grew} bytes" + ); + } + let before = rss_bytes(); + { + let f = LazyFile::open_mmap(&path).unwrap(); + assert_eq!(f.dataset("small").unwrap().read_i32().unwrap(), small); + let grew = rss_bytes().saturating_sub(before); + assert!( + grew < LIMIT, + "LazyFile::open_mmap: resident memory grew {grew} bytes" + ); + } +} From 6559a91495c12b005a434bddadcae091d787fc66 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:46:28 -0500 Subject: [PATCH 16/20] fix: open a file whose cache image cannot load, and fail its objects For a metadata cache image libhdf5 cannot load, libhdf5 opens the file and fails the first metadata read (the image loads on the first H5C_protect after open); h5py reports the error on the root group. The conformance probe reported it that way, but File::open refused the file, so the gate counted cve-2025-6269-1..4 and cve-2025-6516 as agreeing with h5py for behaviour the library did not have. The library now behaves as the probe reports: File (mmap, buffered and from_bytes) and MmapFile open the file and every object lookup (dataset, dataset_at, group, group listings and attributes, VL decoding) fails with the image's error; LazyFile reads the root group's header at open, so its open is that first read and fails. Probe and library take the three-way decision (refuse at open / image loads / image cannot load) from the same clawhdf5_format::superblock_ext::cache_image_state. One deliberate difference from libhdf5 remains, documented: after the failed first read libhdf5 reads the file's own metadata, which the image was meant to replace and may be stale; here every lookup keeps failing. File::cache_image_error / MmapFile::cache_image_error expose the error to code that parses as_bytes() itself; h5rs checks it before reading any object header (h5rs ls on cve-2025-6269-1 said "invalid object header version: 0" from the stale bytes). Test: metadata_cache_image.rs an_image_libhdf5_cannot_load_fails_every_object (the fixture with its image signature broken; h5py opens that file and fails the first read with "Bad metadata cache image header signature"). It fails on the previous commit, where File::open refuses the file. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 12 ++++- conformance/probe/src/main.rs | 10 ++-- crates/clawhdf5-tools/src/h5.rs | 5 ++ crates/clawhdf5/src/lazy.rs | 3 ++ crates/clawhdf5/src/mmap_file.rs | 35 ++++++++++--- crates/clawhdf5/src/reader.rs | 49 ++++++++++++++----- crates/clawhdf5/tests/metadata_cache_image.rs | 46 +++++++++++++++++ docs/known-issues.md | 16 ++++-- 8 files changed, 148 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b0efd6..d256a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,16 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. image, and an abort for an 8 GiB one; `tests/cache_image_memory.rs` guards it.) An image entry that runs past the end of file is refused (libhdf5 checks only its start; the images it writes never do this). A - file whose image libhdf5 cannot load opens in libhdf5 but nothing in it - can be read; `File::open` refuses it. + file whose image libhdf5 cannot load (`cve-2025-6269-*`, `cve-2025-6516`) + opens, as in libhdf5, and every object lookup fails with the image's + error (`File`, `MmapFile`; `LazyFile` reads the root group at open, so + its open fails). libhdf5 fails only its first metadata read and then + reads the file's own, possibly stale, bytes; those are never read here. + An interim version refused such a file at `File::open` while the + conformance probe reported it as libhdf5 does, so the gate counted five + files as agreeing with h5py that the library did not open; probe and + library now take the decision from the same + `superblock_ext::cache_image_state`. - **The superblock extension is decoded at open, as libhdf5 does:** a File Space Info or Metadata Cache Image message libhdf5 cannot decode makes the open fail (`cve-2020-10810`, `cve-2020-10812` were opened). diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 8c25a88..71c3139 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -712,12 +712,14 @@ fn main() { return; } }; - // libhdf5 decodes the superblock extension at open (File::open does - // the same), and loads a metadata cache image over the file's own + // libhdf5 decodes the superblock extension at open (an error refuses + // the file), and loads a metadata cache image over the file's own // metadata. It loads the image only when it first reads metadata — the // root group — so a file whose image it cannot load still opens and - // every object fails; File::open refuses such a file outright. The - // probe records the image's error where libhdf5 reports it. + // that read fails. The library decides all three cases with the same + // `cache_image_state`: `File` and `MmapFile` open such a file and fail + // every object lookup with the image's error, which is what the probe + // records here (on the root group, where libhdf5 reports it). use clawhdf5_format::superblock_ext::{self, CacheImageState}; let state = match guarded(|| superblock_ext::cache_image_state(hdf5, &sb).map_err(e)) { Ok(x) => x, diff --git a/crates/clawhdf5-tools/src/h5.rs b/crates/clawhdf5-tools/src/h5.rs index 1f4354a..5f71a14 100644 --- a/crates/clawhdf5-tools/src/h5.rs +++ b/crates/clawhdf5-tools/src/h5.rs @@ -234,6 +234,11 @@ impl H5 { } pub fn header(&self, addr: u64) -> Result { + // A metadata cache image libhdf5 cannot load: the file opens, and + // every object fails (its bytes may hold stale metadata). + if let Some(e) = self.file.cache_image_error() { + return Err(Error::at(addr, format!("metadata cache image: {e}"))); + } let off = usize::try_from(addr).map_err(|_| Error::at(addr, "address out of range"))?; ObjectHeader::parse(self.data(), off, self.os(), self.ls()) .map_err(|e| Error::at(addr, format!("object header: {e}"))) diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index cb688e9..34f46cc 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -98,6 +98,9 @@ impl LazyFile { let patched = match view { crate::cache_image::ImageView::Plain => None, crate::cache_image::ImageView::Patched(p) => Some(p), + // libhdf5 opens such a file and fails its first metadata read; + // a LazyFile reads the root group's header at open, so the open + // is that read. crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), }; let data = match &patched { diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 0de5b02..040c02f 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -43,6 +43,9 @@ pub struct MmapFile { /// the image's entries land on are copied (see `crate::cache_image`). /// `None` for a file without an image, read straight from the mapping. patched: Option, + /// The file has a metadata cache image libhdf5 cannot load: every + /// object lookup fails with this error (see `File`). + image_error: Option, } impl MmapFile { @@ -61,10 +64,10 @@ impl MmapFile { crate::cache_image::private_view(reader.as_bytes(), base, end, &superblock, || { clawhdf5_io::HDF5Read::private_copy(&reader) })?; - let patched = match view { - crate::cache_image::ImageView::Plain => None, - crate::cache_image::ImageView::Patched(p) => Some(p), - crate::cache_image::ImageView::Unloadable(e) => return Err(e.into()), + let (patched, image_error) = match view { + crate::cache_image::ImageView::Plain => (None, None), + crate::cache_image::ImageView::Patched(p) => (Some(p), None), + crate::cache_image::ImageView::Unloadable(e) => (None, Some(e)), }; Ok(Self { reader, @@ -72,6 +75,7 @@ impl MmapFile { end, superblock, patched, + image_error, }) } @@ -99,7 +103,7 @@ impl MmapFile { /// Resolve a path and return a `MmapDataset` handle. pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.hdf5_bytes(); + let data = self.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -114,7 +118,7 @@ impl MmapFile { /// Resolve a path and return a `MmapGroup` handle. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.hdf5_bytes(); + let data = self.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(MmapGroup { file: self, @@ -129,14 +133,29 @@ impl MmapFile { self.hdf5_bytes() } + /// The error of a metadata cache image libhdf5 cannot load, when the + /// file has one (see [`crate::File::cache_image_error`]). + pub fn cache_image_error(&self) -> Option<&FormatError> { + self.image_error.as_ref() + } + /// Returns a reference to the parsed superblock. pub fn superblock(&self) -> &Superblock { &self.superblock } + /// The bytes to read metadata from; fails for a file whose cache image + /// cannot be loaded. + fn meta(&self) -> Result<&[u8], FormatError> { + match &self.image_error { + Some(e) => Err(e.clone()), + None => Ok(self.hdf5_bytes()), + } + } + fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( - self.hdf5_bytes(), + self.meta()?, address as usize, self.superblock.offset_size, self.superblock.length_size, @@ -257,7 +276,7 @@ impl<'f> MmapGroup<'f> { /// [`group_v2::resolve_group_children`]); dangling, external and /// user-defined links are left out. fn children(&self) -> Result, Error> { - let data = self.file.hdf5_bytes(); + let data = self.file.meta()?; group_v2::resolve_group_children(data, &self.file.superblock, self.address) .map_err(Error::Format) } diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 9f6da0f..023e417 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -63,6 +63,11 @@ struct FileData { /// an image is read straight from the mapping, and an owned buffer has /// the image written into it in place. patched: Option, + /// The file has a metadata cache image libhdf5 cannot load. libhdf5 + /// opens such a file and fails its first metadata read (the image loads + /// then); every object lookup here fails with this error, and no + /// metadata is read from the file's own, possibly stale, bytes. + image_error: Option, } impl FileData { @@ -88,10 +93,10 @@ impl FileData { })? } }; - let patched = match view { - ImageView::Plain => None, - ImageView::Patched(p) => Some(p), - ImageView::Unloadable(e) => return Err(e.into()), + let (patched, image_error) = match view { + ImageView::Plain => (None, None), + ImageView::Patched(p) => (Some(p), None), + ImageView::Unloadable(e) => (None, Some(e)), }; Ok(( Self { @@ -99,6 +104,7 @@ impl FileData { base, end, patched, + image_error, }, superblock, )) @@ -114,6 +120,15 @@ impl FileData { fn len(&self) -> usize { self.as_bytes().len() } + + /// The bytes to read metadata from; fails for a file whose cache image + /// cannot be loaded (see [`Self::image_error`]). + fn meta(&self) -> Result<&[u8], FormatError> { + match &self.image_error { + Some(e) => Err(e.clone()), + None => Ok(self.as_bytes()), + } + } } // --------------------------------------------------------------------------- @@ -200,7 +215,7 @@ impl File { /// /// The path uses `/` separators (e.g., `"group1/values"`). pub fn dataset(&self, path: &str) -> Result, Error> { - let data = self.data.as_bytes(); + let data = self.data.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; let hdr = self.parse_header(addr)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -235,7 +250,7 @@ impl File { /// The path uses `/` separators (e.g., `"sensors"`). /// Use `"/"` or `""` for the root group. pub fn group(&self, path: &str) -> Result, Error> { - let data = self.data.as_bytes(); + let data = self.data.meta()?; let addr = group_v2::resolve_path_any(data, &self.superblock, path)?; Ok(Group { file: self, @@ -291,11 +306,23 @@ impl File { /// Returns the file's bytes from the superblock on (after any user /// block). Every HDF5 address in the file indexes this slice, so it is - /// what the `clawhdf5_format` parsers expect as `file_data`. + /// what the `clawhdf5_format` parsers expect as `file_data`. For a file + /// with a metadata cache image these are the bytes with the image + /// applied; when the image cannot be loaded they are the file's own + /// bytes, whose metadata may be stale (every object lookup fails then). pub fn as_bytes(&self) -> &[u8] { self.data.as_bytes() } + /// The error of a metadata cache image libhdf5 cannot load, when the + /// file has one. Such a file opens, as in libhdf5, and every object + /// lookup fails with this error; code that parses [`Self::as_bytes`] + /// itself should check it first, since those bytes then hold the + /// file's own, possibly stale, metadata. + pub fn cache_image_error(&self) -> Option<&FormatError> { + self.data.image_error.as_ref() + } + /// Size of the user block before the superblock (0 for most files). /// Matches h5py's `File.userblock_size`. pub fn user_block_size(&self) -> u64 { @@ -340,7 +367,7 @@ impl File { raw: &[u8], ) -> Result>, Error> { crate::vlen::decode_string_bytes( - self.as_bytes(), + self.data.meta()?, datatype, raw, self.offset_size(), @@ -358,7 +385,7 @@ impl File { raw: &[u8], ) -> Result>, Error> { crate::vlen::decode_vlen( - self.as_bytes(), + self.data.meta()?, datatype, raw, self.offset_size(), @@ -368,7 +395,7 @@ impl File { fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( - self.data.as_bytes(), + self.data.meta()?, address as usize, self.superblock.offset_size, self.superblock.length_size, @@ -490,7 +517,7 @@ impl<'f> Group<'f> { /// [`group_v2::resolve_group_children`]); dangling, external and /// user-defined links are left out. fn children(&self) -> Result, Error> { - let data = self.file.data.as_bytes(); + let data = self.file.data.meta()?; group_v2::resolve_group_children(data, &self.file.superblock, self.address) .map_err(Error::Format) } diff --git a/crates/clawhdf5/tests/metadata_cache_image.rs b/crates/clawhdf5/tests/metadata_cache_image.rs index 3cbc496..92463e8 100644 --- a/crates/clawhdf5/tests/metadata_cache_image.rs +++ b/crates/clawhdf5/tests/metadata_cache_image.rs @@ -46,3 +46,49 @@ fn mmap_and_lazy_files_read_through_the_cache_image() { expected() ); } + +/// A file whose cache image libhdf5 cannot load: libhdf5 opens it, and the +/// first metadata read fails with the image's error (h5py: +/// `Unable to get group info (Bad metadata cache image header signature)`); +/// later reads get the file's own bytes, which here are stale (the root +/// group's header is zeros: "bad object header version number"). The +/// openers agree on the open and the failure: `File` and `MmapFile` open +/// and fail every object lookup with the image's error (never reading the +/// stale bytes), `LazyFile` reads the root group's header at open and so +/// fails there. `File::open` used to refuse the file, while the +/// conformance probe reported it as h5py does. +#[test] +fn an_image_libhdf5_cannot_load_fails_every_object() { + let mut bytes = std::fs::read(fixture()).unwrap(); + let at = bytes.windows(4).position(|w| w == b"MDCI").unwrap(); + bytes[at] = b'X'; + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("bad_image.h5"); + std::fs::write(&path, &bytes).unwrap(); + + let is_image_error = |e: clawhdf5::Error| { + matches!( + e, + clawhdf5::Error::Format(clawhdf5_format::error::FormatError::InvalidCacheImage( + "bad metadata cache image header signature" + )) + ) + }; + for file in [ + File::open(&path).unwrap(), + File::from_bytes(bytes.clone()).unwrap(), + ] { + assert!(is_image_error(file.root().datasets().unwrap_err())); + assert!(is_image_error(file.root().attrs().unwrap_err())); + assert!(is_image_error(file.dataset("DSET").unwrap_err())); + assert!(is_image_error(file.group("/").map(|_| ()).unwrap_err())); + assert!(is_image_error(file.dataset_at(96).unwrap_err())); + } + let mm = MmapFile::open(&path).unwrap(); + assert!(is_image_error(mm.root().datasets().unwrap_err())); + assert!(is_image_error(mm.dataset("DSET").unwrap_err())); + assert!(is_image_error(mm.group("/").map(|_| ()).unwrap_err())); + assert!(is_image_error( + LazyFile::from_bytes(bytes).map(|_| ()).unwrap_err() + )); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index b7c471a..8328f14 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -205,9 +205,19 @@ fill-value item that did is fixed). - Metadata cache images are not supported. **Fixed 2026-09-26:** the image is applied at open, as libhdf5 loads it over the file's metadata (`clawhdf5_format::superblock_ext`); `h5clear_mdc_image.h5` reads - (`crates/clawhdf5/tests/metadata_cache_image.rs`). A file whose image - libhdf5 cannot load (`cve-2025-6269-*`, `cve-2025-6516`) opens in - libhdf5 with nothing readable in it; `File::open` refuses it. + (`crates/clawhdf5/tests/metadata_cache_image.rs`), without copying the + file (a private copy-on-write mapping takes the image's entries; + `tests/cache_image_memory.rs`). A file whose image libhdf5 cannot load + (`cve-2025-6269-*`, `cve-2025-6516`) opens, as in libhdf5, and every + object lookup fails with the image's error. Differences from libhdf5 + that remain: libhdf5 fails only the first metadata read and then reads + the file's own (possibly stale) metadata, where we keep failing; an + image entry that runs past the end of file is refused (libhdf5 checks + only its start); a flush-dependency parent flag is checked against the + child count as libhdf5's debug build checks it (HDF5 2.0 release + builds refuse every entry that has children, even in images they + wrote); the superblock extension's driver-info and shared-message table + messages are not decoded at open. - x87 long double and binary128 are refused. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail. **Not our bug (checked 2026-09-26):** both are corrupt files HDF5 2.0 From d493d4792e1d711cdd4746ca9fa101e79246ab81 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:49:13 -0500 Subject: [PATCH 17/20] fix: apply the superblock extension and cache image in every opener File, MmapFile and LazyFile decoded the superblock extension and laid a metadata cache image over the file's metadata; the other readers did not, so the same file read differently by entry point: NativeVol, AsyncHDF5File and MpiVol (clawhdf5-io) and the external source files of a virtual dataset (clawhdf5-format vds.rs) read a file with an image from its own bytes, which libhdf5 does not (they may be stale, or zeros: h5clear_mdc_image.h5 failed with InvalidObjectHeaderVersion(0)), and skipped the extension checks File::open makes (cve-2020-10810/10812). Each of them owns its buffer, so each now calls the shared superblock_ext::apply_cache_image_in_place, which checks the extension and writes the image's entries in place (only the image block is copied). These readers read whole datasets and cannot open a file and fail each object, so an image libhdf5 cannot load is refused with the image's error, never read around. clawhdf5-io's vol::load_hdf5 wraps it for NativeVol (at open; for from_bytes the error is reported on read, as a truncated file already was) and MpiVol. The MpiVol edit is minimal and was not compiled: the mpi-io feature needs an MPI installation this machine does not have (mpi-sys's build script panics). Tests: NativeVol (open_path and from_bytes), AsyncHDF5File and a VDS whose source file is h5clear_mdc_image.h5 (vds_interop.rs, against h5py) read the fixture's values; the corrupted-image variants are refused. Each fails without its fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 12 +++++ crates/clawhdf5-format/src/vds.rs | 23 +++++++- crates/clawhdf5-io/src/async_read.rs | 41 +++++++++++++++ crates/clawhdf5-io/src/mpi_vol.rs | 5 +- crates/clawhdf5-io/src/vol.rs | 78 ++++++++++++++++++++++++++-- crates/clawhdf5/tests/vds_interop.rs | 23 ++++++++ 6 files changed, 176 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d256a15..8912fed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,18 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. files as agreeing with h5py that the library did not open; probe and library now take the decision from the same `superblock_ext::cache_image_state`. +- **Every other opener applies the superblock extension and the cache + image too** (`superblock_ext::apply_cache_image_in_place`, writing into + the buffer each already owns): `clawhdf5_io`'s `NativeVol` (at `open`, + and on read for `from_bytes`), `AsyncHDF5File`, `MpiVol` (a minimal edit + through the same `vol::load_hdf5`; the `mpi-io` feature cannot be built + without an MPI installation, so it was not compiled), and the external + source files of a virtual dataset. They read a file with an image from + its own bytes — stale metadata, or none (`h5clear_mdc_image.h5` failed + with `InvalidObjectHeaderVersion(0)`) — and skipped the extension checks + `File::open` makes. These readers cannot open a file and fail each + object, so an image libhdf5 cannot load is refused with the image's + error. - **The superblock extension is decoded at open, as libhdf5 does:** a File Space Info or Metadata Cache Image message libhdf5 cannot decode makes the open fail (`cve-2020-10810`, `cve-2020-10812` were opened). diff --git a/crates/clawhdf5-format/src/vds.rs b/crates/clawhdf5-format/src/vds.rs index f5d5237..d67b832 100644 --- a/crates/clawhdf5-format/src/vds.rs +++ b/crates/clawhdf5-format/src/vds.rs @@ -803,7 +803,11 @@ impl<'a, 'r> Sources<'a, 'r> { let resolver = self.resolver.ok_or_else(|| { vds_err("external-file virtual dataset sources require a file resolver") })?; - self.cached_file = Some((String::from(name), resolver(name)?)); + let mut bytes = resolver(name)?; + if let Some(b) = bytes.as_mut() { + load_source_file(b)?; + } + self.cached_file = Some((String::from(name), bytes)); } // An external file is handed over whole; its addresses are relative // to its superblock, so skip any user block. @@ -851,6 +855,23 @@ impl<'a, 'r> Sources<'a, 'r> { } } +/// Check an external source file's superblock extension as libhdf5 does +/// when it opens the file, and write any metadata cache image over its +/// metadata in place: libhdf5 reads the image's entries instead of the +/// file's own, possibly stale, bytes (`crate::superblock_ext`). A source +/// file whose image cannot be loaded is an error, as other corrupt source +/// files are here. +fn load_source_file(whole: &mut [u8]) -> Result<(), FormatError> { + let base = crate::signature::find_signature(whole)?; + let sb = crate::superblock::Superblock::parse(&whole[base..], 0)?; + // The end of file the superblock records; a truncated source file is + // read as before, up to its length. + let end = sb + .data_end(base as u64, whole.len() as u64) + .map_or(whole.len(), |e| base + e as usize); + crate::superblock_ext::apply_cache_image_in_place(&mut whole[base..end], &sb) +} + /// Whether elements of `dt` contain addresses into their own file: /// variable-length data (global-heap IDs) or references. fn holds_file_addresses(dt: &Datatype) -> bool { diff --git a/crates/clawhdf5-io/src/async_read.rs b/crates/clawhdf5-io/src/async_read.rs index 30969fa..f5277c0 100644 --- a/crates/clawhdf5-io/src/async_read.rs +++ b/crates/clawhdf5-io/src/async_read.rs @@ -288,6 +288,12 @@ impl AsyncHDF5File { // superblock records, as libhdf5 does. let end = superblock.data_end(user_block as u64, whole_len)?; data.truncate(end as usize); + // Check the superblock extension as libhdf5 does at open, and write + // any metadata cache image over the file's metadata (libhdf5 reads + // the image's entries instead of the file's own, possibly stale, + // bytes). An image libhdf5 cannot load is refused: this reader has + // no way to open the file and fail each object instead. + clawhdf5_format::superblock_ext::apply_cache_image_in_place(&mut data, &superblock)?; Ok(Self { data, superblock }) } @@ -430,6 +436,41 @@ mod tests { fw.finish().unwrap() } + /// libhdf5's `h5clear_mdc_image.h5` (see the clawhdf5 crate's + /// `tests/metadata_cache_image.rs`): the root group's header exists + /// only in the file's metadata cache image. + fn cache_image_fixture() -> Vec { + std::fs::read(concat!( + env!("CARGO_MANIFEST_DIR"), + "/../clawhdf5/tests/fixtures/h5clear_mdc_image.h5" + )) + .unwrap() + } + + #[tokio::test] + async fn reads_through_a_metadata_cache_image() { + let bytes = cache_image_fixture(); + let file = AsyncHDF5File::from_bytes(bytes.clone()).unwrap(); + let info = file.read_dataset_raw("DSET").await.unwrap(); + assert_eq!(info.shape, [50, 100]); + let values: Vec = info + .raw + .chunks_exact(4) + .map(|b| i32::from_le_bytes(b.try_into().unwrap())) + .collect(); + let expected: Vec = (0..50).flat_map(|i| (0..100).map(move |j| i * j)).collect(); + assert_eq!(values, expected); + + // An image libhdf5 cannot load is refused at open. + let mut bad = bytes; + let at = bad.windows(4).position(|w| w == b"MDCI").unwrap(); + bad[at] = b'X'; + assert!(matches!( + AsyncHDF5File::from_bytes(bad), + Err(AsyncHDF5Error::Format(FormatError::InvalidCacheImage(_))) + )); + } + // --- AsyncMemoryReader tests --- #[tokio::test] diff --git a/crates/clawhdf5-io/src/mpi_vol.rs b/crates/clawhdf5-io/src/mpi_vol.rs index 87a82ea..fdf4d56 100644 --- a/crates/clawhdf5-io/src/mpi_vol.rs +++ b/crates/clawhdf5-io/src/mpi_vol.rs @@ -191,7 +191,10 @@ fn mpi_collective_read(vol: &MpiVol, location: &str, path: &str) -> Result>, location: Option, + /// Why the bytes given to [`NativeVol::from_bytes`] cannot be read + /// (what `open` would have refused them for). + load_error: Option, } /// The HDF5 bytes of a whole file and its superblock: from the superblock @@ -228,12 +231,35 @@ pub(crate) fn hdf5_view( Ok((&data[..end as usize], sb)) } +/// Check a whole file as libhdf5 does when it opens it ([`hdf5_view`], and +/// the superblock extension), and write any metadata cache image over the +/// file's metadata in place: libhdf5 reads the image's entries instead of +/// the file's own bytes at their addresses, which may be stale +/// (`clawhdf5_format::superblock_ext`). A file whose image libhdf5 cannot +/// load is refused: a connector that reads whole datasets has no way to +/// open the file and fail each object instead. +pub(crate) fn load_hdf5(whole: &mut [u8]) -> Result<(), VolError> { + use clawhdf5_format::superblock_ext::apply_cache_image_in_place; + let err = |e: clawhdf5_format::error::FormatError| VolError::DataError(e.to_string()); + let (len, sb) = { + let (data, sb) = hdf5_view(whole)?; + (data.len(), sb) + }; + // hdf5_view's bytes start at the superblock, after any user block. + let base = clawhdf5_format::signature::split_user_block(whole) + .map_err(err)? + .0 + .len(); + apply_cache_image_in_place(&mut whole[base..base + len], &sb).map_err(err) +} + impl NativeVol { /// Create a new native VOL connector. pub fn new() -> Self { Self { data: None, location: None, + load_error: None, } } @@ -245,10 +271,12 @@ impl NativeVol { } /// Create a native VOL connector from bytes already in memory. - pub fn from_bytes(data: Vec) -> Self { + pub fn from_bytes(mut data: Vec) -> Self { + let load_error = load_hdf5(&mut data).err().map(|e| e.to_string()); Self { data: Some(data), location: Some("".into()), + load_error, } } @@ -281,10 +309,12 @@ impl VirtualObjectLayer for NativeVol { } fn open(&mut self, location: &str) -> Result<(), VolError> { - let data = std::fs::read(location)?; - // Refuse a truncated file at open, as libhdf5 does. - hdf5_view(&data)?; + let mut data = std::fs::read(location)?; + // Refuse a truncated file at open, as libhdf5 does, and load any + // metadata cache image. + load_hdf5(&mut data)?; self.data = Some(data); + self.load_error = None; self.location = Some(location.to_string()); Ok(()) } @@ -299,6 +329,9 @@ impl VirtualObjectLayer for NativeVol { let data = self.data.as_ref().ok_or_else(|| { VolError::Io(io::Error::new(io::ErrorKind::NotConnected, "file not open")) })?; + if let Some(e) = &self.load_error { + return Err(VolError::DataError(e.clone())); + } use clawhdf5_format::{ data_layout::DataLayout, data_read::read_raw_data_full, dataspace::Dataspace, @@ -434,6 +467,43 @@ mod tests { assert_eq!(raw.len(), 24); } + /// libhdf5's `h5clear_mdc_image.h5` (see the clawhdf5 crate's + /// `tests/metadata_cache_image.rs`): the root group's header exists + /// only in the file's metadata cache image, so reading the file's own + /// bytes finds zeros there. + #[test] + fn native_vol_reads_through_a_metadata_cache_image() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../clawhdf5/tests/fixtures/h5clear_mdc_image.h5" + ); + let expected: Vec = (0..50) + .flat_map(|i| (0..100).map(move |j| i * j)) + .flat_map(i32::to_le_bytes) + .collect(); + let vol = NativeVol::open_path(path).unwrap(); + assert_eq!(vol.read_dataset("DSET").unwrap(), expected); + let bytes = std::fs::read(path).unwrap(); + let vol = NativeVol::from_bytes(bytes.clone()); + assert_eq!(vol.read_dataset("DSET").unwrap(), expected); + + // An image libhdf5 cannot load is refused, not read around. + let mut bad = bytes; + let at = bad.windows(4).position(|w| w == b"MDCI").unwrap(); + bad[at] = b'X'; + let err = NativeVol::from_bytes(bad.clone()) + .read_dataset("DSET") + .unwrap_err(); + assert!(err.to_string().contains("cache image"), "{err}"); + let dir = tempfile::tempdir().unwrap(); + let bad_path = dir.path().join("bad_image.h5"); + std::fs::write(&bad_path, &bad).unwrap(); + let err = NativeVol::open_path(bad_path.to_str().unwrap()) + .err() + .unwrap(); + assert!(err.to_string().contains("cache image"), "{err}"); + } + #[test] fn vol_error_display() { let err = VolError::Unsupported("read_dataset".into()); diff --git a/crates/clawhdf5/tests/vds_interop.rs b/crates/clawhdf5/tests/vds_interop.rs index 17069b0..71e512e 100644 --- a/crates/clawhdf5/tests/vds_interop.rs +++ b/crates/clawhdf5/tests/vds_interop.rs @@ -486,3 +486,26 @@ fn vds_libhdf5_test_files() { vec![5, 10, 10] ); } + +/// A source file with a metadata cache image (libhdf5's +/// `h5clear_mdc_image.h5`, whose root group's header exists only in the +/// image) is read through the image, as libhdf5 opens it. Source files were +/// read from their own bytes, and this one failed on the root group. +#[test] +fn vds_source_file_with_a_metadata_cache_image() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let fixture = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/h5clear_mdc_image.h5"); + std::fs::copy(&fixture, dir.path().join("src.h5")).unwrap(); + generate( + dir.path(), + r#" +with h5py.File("vds.h5", "w", libver="latest") as f: + lay = h5py.VirtualLayout(shape=(3, 100), dtype="i4") + lay[0:3, :] = h5py.VirtualSource("src.h5", "DSET", shape=(50, 100))[5:8, :] + f.create_virtual_dataset("v", lay) +expect("vds.h5", "v", "image_source") +"#, + ); + assert_matches_libhdf5(dir.path(), "vds.h5", "v", "image_source"); +} From 4c01267b76c05fc36a8395d9df6f1155f74efc69 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:51:20 -0500 Subject: [PATCH 18/20] test: every scale-offset dataset h5py writes reads as h5py reads it The scale-offset fix (d110b1d) was covered only by unit vectors from CVE chunks, and documented as three corner cases. The review found it is much bigger: of 1480 scale-offset datasets h5py writes (every integer type i1..u8, f4 and f8, both byte orders, with and without a fill value, scaleoffset 0..full width), v2.7.0's decoder read 332 differently from h5py: 151 returned wrong values with no error (82 integer datasets with scaleoffset=0 and a wide range, 51 full-width i4/u4/i8/u8, 18 f4 D-scale datasets with a large range) and 181 failed to read. The cause in every case is a chunk libhdf5 stores at full width, whose elements were decoded as offsets from minval. tests/scaleoffset_interop.rs generates that matrix with h5py at test time, stores h5py's decoded values uncompressed next to it, and compares every dataset's bytes. It passes on this branch; with the filters.rs before d110b1d it reports "332 of 1480 scale-offset datasets differ from h5py". CHANGELOG: a Correctness entry stating this was silent wrong data in every release that decoded scale-offset (v2.2.0 to v2.7.0), replacing the corner-case wording. docs/known-issues.md: a fixed entry with the affected cases. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 32 +++- crates/clawhdf5/tests/scaleoffset_interop.rs | 151 +++++++++++++++++++ docs/known-issues.md | 40 ++++- 3 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 crates/clawhdf5/tests/scaleoffset_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8912fed..253e9a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,12 +61,10 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. - a simple dataspace of rank 0 holds one element (it held 0; `cve-2020-18494`), and contiguous storage larger than the dataset reads (`cve-2024-32623`, `cve-2025-2309`; libhdf5 ignores the excess); - - scale-offset: the packed codes start at byte 21 whatever size the chunk - records for `minval`, a chunk with `minbits` 0 and a fill value is all - fill values, full-width `minbits` stores the elements as they are - (these decoded differently from libhdf5); E-scale is refused, as in - libhdf5; codes past the end of the chunk stay an error - (`cve-2025-2308`, where HDF5 2.0 reads past its buffer); + - scale-offset returned wrong values for ordinary h5py files — see + *Correctness* below; E-scale is refused, as in libhdf5; codes past the + end of the chunk stay an error (`cve-2025-2308`, where HDF5 2.0 reads + past its buffer); - shuffle uses its own parameter as the element size, as libhdf5 does (`cve-2025-44905`); - an unfiltered chunk the index records at other than the chunk's size is @@ -878,6 +876,28 @@ ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. - CI keeps zlib-ng building and tested; the arm64 job no longer needs cmake. ### Correctness +- **Scale-offset data read wrong values in every release that decoded it + (v2.2.0 to v2.7.0), silently, on ordinary h5py files** (fixed + 2026-09-26). Of 1480 scale-offset datasets h5py writes across every + integer type (`i1` .. `u8`), `f4` and `f8`, both byte orders, with and + without a fill value, and `scaleoffset` from 0 to the full width, 332 did + not read as h5py reads them: **151 returned wrong values with no error** + and 181 failed to read. The common cause was a chunk libhdf5 stores at + full width (`minbits` equal to the type's width), which it does for any + full-width `scaleoffset` and on its own whenever a chunk's values span + most of the type's range: `scaleoffset=0` integer data with a wide range + (82 datasets, all wrong values), full-width `u4`/`i4`/`u8`/`i8` (51 wrong + values; the narrower types and the rest failed with "truncated minval" or + "implausible minbits"), and `f4` D-scale data with a large range (18, + wrong values). Such a chunk holds the elements as they are; they were + decoded as offsets from `minval`. Also fixed, found on crafted files: the + packed codes start at byte 21 whatever size the chunk records for + `minval` (`cve-2025-44905` `/Scale_offset_short_data_be`), and a chunk + with `minbits` 0 and a fill value is all fill values (it read as + `minval`). The whole matrix is now an interop test + (`crates/clawhdf5/tests/scaleoffset_interop.rs`, generated by h5py at + test time, every dataset compared); on v2.7.0's decoder it reports the + 332. See `docs/known-issues.md`. - **Corrupt files libhdf5 refuses are now refused instead of read.** On the HDF Group's CVE reproducers, 18 objects that libhdf5 (HDF5 2.0, through h5py) refuses to open were read by clawhdf5, some as wrong data (a chunk diff --git a/crates/clawhdf5/tests/scaleoffset_interop.rs b/crates/clawhdf5/tests/scaleoffset_interop.rs new file mode 100644 index 0000000..d31c0db --- /dev/null +++ b/crates/clawhdf5/tests/scaleoffset_interop.rs @@ -0,0 +1,151 @@ +//! Scale-offset datasets written by h5py (libhdf5) read exactly as h5py +//! reads them. +//! +//! h5py writes the whole matrix the filter has: every integer type (`i1` .. +//! `u8`) and `f4`/`f8`, both byte orders, with and without a fill value +//! (the type's minimum, maximum, or another value), with random, constant, +//! all-fill and extreme data, and every interesting `scaleoffset` setting +//! (0 = let libhdf5 choose the bits, a few bits, full width less one, full +//! width; decimal scale factors 0..7 for floats) — about 1480 datasets. For +//! each one h5py's decoded values are stored uncompressed next to it, and +//! the raw bytes clawhdf5 decodes must equal them. +//! +//! Until 2026-09-26 clawhdf5 silently returned wrong values for 332 of +//! these, in every release that decoded scale-offset: ordinary `u8`, `u4`, +//! `i8` and `f4` data with a wide range (where libhdf5 stores the elements +//! as they are, at full width), chunks whose `minval` field was recorded at +//! another size than 8 bytes, and chunks with `minbits` 0 and a fill value. +//! +//! Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::process::Command; + +use clawhdf5::File; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +const GENERATE: &str = r#" +import sys, h5py, numpy as np +rng = np.random.default_rng(7) +out, expect = sys.argv[1], sys.argv[2] +made = [] +with h5py.File(out, "w") as f: + def make(name, a, so, fv, desc): + try: + d = f.create_dataset(name, data=a, chunks=(16,), scaleoffset=so, fillvalue=fv) + except Exception: + return # a combination libhdf5 refuses to write + d.attrs["desc"] = desc + made.append(name) + i = 0 + for dt in ["i1", "u1", "i2", "u2", "i4", "u4", "i8", "u8"]: + for bo in "<>": + t = np.dtype(bo + dt) + info = np.iinfo(t) + for so in [0, 1, 3, 8 * t.itemsize - 1, 8 * t.itemsize]: + for fill in [None, "min", "max", "mid"]: + for pat in ["rand", "const", "allfill", "extreme"]: + n = 64 + fv = {None: None, "min": info.min, "max": info.max, "mid": t.type(7)}[fill] + if pat == "rand": + lo = max(info.min, -50) if so else info.min + hi = min(info.max, 50) if so else info.max + a = rng.integers(lo, hi, size=n, endpoint=True, + dtype=np.int64 if t.kind == "i" else np.uint64).astype(t) + elif pat == "const": + a = np.full(n, 3, t) + elif pat == "extreme": + a = np.array([info.min, info.max] * (n // 2), t) + else: + if fv is None: + continue + a = np.full(n, fv, t) + make(f"d{i}", a, so, fv, f"{bo}{dt} so={so} fill={fill} pat={pat}") + i += 1 + for dt in ["f4", "f8"]: + for bo in "<>": + t = np.dtype(bo + dt) + for so in [0, 1, 2, 4, 7]: + for fill in [None, -1.5, 0.0]: + for pat in ["rand", "const", "allfill", "neg", "big"]: + n = 50 + if pat == "rand": + a = rng.normal(size=n).astype(t) * 10 + elif pat == "const": + a = np.full(n, 2.25, t) + elif pat == "neg": + a = -np.abs(rng.normal(size=n)).astype(t) * 1000 + elif pat == "big": + a = rng.normal(size=n).astype(t) * 1e6 + else: + if fill is None: + continue + a = np.full(n, fill, t) + make(f"d{i}", a, so, fill, f"{bo}{dt} so={so} fill={fill} pat={pat}") + i += 1 +# What libhdf5 decodes, stored uncompressed in the same datatype. +with h5py.File(out, "r") as f, h5py.File(expect, "w") as e: + for name in made: + e.create_dataset(name, data=f[name][()]) +print(len(made)) +"#; + +#[test] +fn every_scale_offset_dataset_reads_as_h5py_reads_it() { + if !python_available() { + assert!( + !std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1"), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + let dir = tempfile::tempdir().unwrap(); + let written = dir.path().join("scaleoffset.h5"); + let expected = dir.path().join("expected.h5"); + let out = Command::new(python()) + .args(["-c", GENERATE]) + .arg(&written) + .arg(&expected) + .output() + .expect("failed to run python"); + assert!( + out.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + let count: usize = String::from_utf8_lossy(&out.stdout).trim().parse().unwrap(); + assert!(count > 1400, "only {count} datasets written"); + + let file = File::open(&written).unwrap(); + let reference = File::open(&expected).unwrap(); + let mut names = reference.root().datasets().unwrap(); + names.sort(); + assert_eq!(names.len(), count); + let mut wrong = Vec::new(); + for name in &names { + let want = reference.read_multi(&[name]).unwrap().remove(0); + let got = file.read_multi(&[name]).map(|mut v| v.remove(0)); + if got.as_ref().ok() != Some(&want) { + let desc = file.dataset(name).unwrap().attrs().unwrap().remove("desc"); + wrong.push(format!("{name} {desc:?}: {:?}", got.map(|g| g.len()))); + } + } + assert!( + wrong.is_empty(), + "{} of {count} scale-offset datasets differ from h5py:\n{}", + wrong.len(), + wrong.join("\n") + ); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 8328f14..5d1aac7 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -63,6 +63,43 @@ tank with `concurrent_read` against h5py 3.16 / HDF5 2.0 (`BENCHMARKS.md`, `CHANGELOG.md`). The chunked-read scaling item above is still open. Values are correct; this is speed only. +## Scale-offset data read back wrong values + +**Status:** fixed 2026-09-26, after v2.7.0. **Every release that decoded +the scale-offset filter (v2.2.0 to v2.7.0) is affected**, on ordinary +files h5py writes, with no error. + +Found by the review of the 2026-09-26 conformance work: h5py wrote 1480 +scale-offset datasets (every integer type `i1` .. `u8`, `f4` and `f8`, +little- and big-endian, with no fill value and with the type's minimum, +maximum or another value as fill, random, constant, all-fill and extreme +data, `scaleoffset` 0, 1, 3, full width less one and full width for +integers, decimal scale factors 0, 1, 2, 4 and 7 for floats). v2.7.0's +decoder read 332 of them differently from h5py: **151 returned wrong +values with no error**, 181 failed to read. + +| Case | Datasets | v2.7.0 | +|---|---|---| +| integer, `scaleoffset=0` (libhdf5 picks the bits), data spanning most of the type's range | 82 | wrong values | +| integer, `scaleoffset` = full width: `i4`/`u4` (10), `i8`/`u8` (41) | 51 | wrong values | +| integer, `scaleoffset` = full width, the other datasets (every `i1`..`u2` one, most `i4`/`u4`, some `i8`/`u8`) | 181 | "truncated minval" / "implausible minbits" | +| `f4` D-scale, factor 4 or 7, values up to about 10^6 | 18 | wrong values | + +In every case libhdf5 stored a chunk at full width (`minbits` equal to the +type's width): the chunk then holds the elements as they are, and they +were decoded as offsets from `minval`. Two more differences were found on +crafted files and fixed with them: the packed codes start at byte 21 +whatever size the chunk records for `minval` (`cve-2025-44905` +`/Scale_offset_short_data_be`), and a chunk with `minbits` 0 and a fill +value is all fill values (it read as `minval`). + +**Fix:** `clawhdf5_format::filters` decodes a scale-offset chunk as +`H5Z__filter_scaleoffset` does. **Test:** the whole matrix is +`crates/clawhdf5/tests/scaleoffset_interop.rs`, generated by h5py at test +time and compared dataset by dataset; on v2.7.0's decoder it reports the +332. **Existing data:** the files were always right; only reads were +wrong, so re-reading with a fixed build gives the correct values. + ## Silent wrong data found by the 2026-09-25 HDF5 audit **Status:** fixed after v2.7.0 (2026-09-25). **Every release up @@ -230,7 +267,8 @@ fill-value item that did is fixed). under *Known not-our-bug*. Scale-offset did decode three cases differently from libhdf5 (codes after a `minval` of recorded size other than 8, `minbits` 0 with a fill value, full-width `minbits`): fixed - 2026-09-26. + 2026-09-26, and the full-width case was silent wrong data on ordinary + h5py files (see *Scale-offset data read back wrong values* above). - **Filters:** blosc, blosc2, bitshuffle, bzip2, LZF and zfp are not implemented. **Fixed 2026-09-26** for LZF (default-on `lzf` feature), bitshuffle, bzip2 and Blosc 1 (`bitshuffle`, `bzip2`, `blosc`, or From 00f94d57eddddeed8736259e40a86fbac52efdcd Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:52:01 -0500 Subject: [PATCH 19/20] test(io): read the cache-image fixture's values with as_chunks clippy (with the async feature) flags chunks_exact with a constant size in the AsyncHDF5File cache-image test added in d493d47. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-io/src/async_read.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/clawhdf5-io/src/async_read.rs b/crates/clawhdf5-io/src/async_read.rs index f5277c0..fa87355 100644 --- a/crates/clawhdf5-io/src/async_read.rs +++ b/crates/clawhdf5-io/src/async_read.rs @@ -455,8 +455,10 @@ mod tests { assert_eq!(info.shape, [50, 100]); let values: Vec = info .raw - .chunks_exact(4) - .map(|b| i32::from_le_bytes(b.try_into().unwrap())) + .as_chunks::<4>() + .0 + .iter() + .map(|&b| i32::from_le_bytes(b)) .collect(); let expected: Vec = (0..50).flat_map(|i| (0..100).map(move |j| i * j)).collect(); assert_eq!(values, expected); From 55e0e7e9cf2ede678122fb5a08a783c66ba5ae9b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 11:53:46 -0500 Subject: [PATCH 20/20] docs: conformance numbers after the review fixes (598 of 697 ok) cve-2025-44905 now reads as h5py reads it (the v1 chunk B-tree lookup), leaving 5 our-errors: cve-2025-2308, cve-2025-44904 and bad_nbit_parms_walk (corrupt data HDF5 2.0 reads through a bug), and the Blosc2 and ZFP filters. The five unloadable-cache-image files stay ok, now with the library behaving as the probe reports. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 253e9a0..da05b6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,13 @@ ## Unreleased ### Remaining conformance errors (2026-09-26) -Conformance on tank, `conformance/run.sh --no-fetch`: 597 of 697 files -ok (575 before). Of the 6 our-errors left, 4 are corrupt data HDF5 2.0 +Conformance on tank, `conformance/run.sh --no-fetch`: 598 of 697 files +ok (575 before). Of the 5 our-errors left, 3 are corrupt data HDF5 2.0 reads only through a bug (listed in `CONFORMANCE.md`), 2 are the Blosc2 and -ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. +ZFP filters; the 2 mismatches are the known h5py big-endian VL bug. The +five files whose cache image libhdf5 cannot load (`cve-2025-6269-*`, +`cve-2025-6516`) count as ok because the library, like libhdf5, opens them +and fails their objects (see below). - **Metadata cache images are read.** A file written with a metadata cache image keeps its metadata cache entries in an image block the superblock extension points at, and libhdf5 reads them in place of the file's own