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" + ); + } +}