From 32fb6689585475e42709c35d61a5a1ce18c89ce0 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 29 Sep 2026 11:24:23 +0200 Subject: [PATCH 1/2] aboot parsing: Minor cleanup Avoid indexing arrays when parsing the split cmdline, instead use CStr::from_bytes_until_nul() and iterators. Signed-off-by: Alexander Larsson --- crates/composefs-boot/src/android_boot.rs | 40 ++++++++++++++++------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/crates/composefs-boot/src/android_boot.rs b/crates/composefs-boot/src/android_boot.rs index 7b73a7b8..ef7bf93a 100644 --- a/crates/composefs-boot/src/android_boot.rs +++ b/crates/composefs-boot/src/android_boot.rs @@ -3,6 +3,7 @@ //! This module provides functionality to parse Android boot image format version 2 files //! and extract embedded components like kernel, initrd, commandline and dtb. +use std::ffi::CStr; use std::io::{Read, Seek, SeekFrom}; use thiserror::Error; use zerocopy::{ @@ -115,12 +116,12 @@ impl AndroidBootImage { } // mkbootimg splits long command lines (with a null terminator for each) - let primary_len = nul_terminated_len(&header.cmdline); - let extra_len = nul_terminated_len(&header.extra_cmdline); + let primary = nul_terminated_bytes(&header.cmdline); + let extra = nul_terminated_bytes(&header.extra_cmdline); let mut cmdline = [0; TOTAL_CMDLINE_SIZE]; - cmdline[..primary_len].copy_from_slice(&header.cmdline[..primary_len]); - cmdline[primary_len..primary_len + extra_len] - .copy_from_slice(&header.extra_cmdline[..extra_len]); + for (dst, src) in cmdline.iter_mut().zip(primary.iter().chain(extra)) { + *dst = *src; + } Ok(Self { page_size, @@ -179,16 +180,12 @@ impl AndroidBootImage { /// Return the kernel command line stored in the image header. pub fn cmdline(&self) -> Result<&str, AndroidBootError> { - let end = nul_terminated_len(&self.cmdline); - Ok(std::str::from_utf8(&self.cmdline[..end])?) + Ok(std::str::from_utf8(nul_terminated_bytes(&self.cmdline))?) } } -fn nul_terminated_len(bytes: &[u8]) -> usize { - bytes - .iter() - .position(|byte| *byte == 0) - .unwrap_or(bytes.len()) +fn nul_terminated_bytes(bytes: &[u8]) -> &[u8] { + CStr::from_bytes_until_nul(bytes).map_or(bytes, CStr::to_bytes) } fn add_aligned( @@ -305,6 +302,25 @@ pub(crate) mod tests { Ok(()) } + #[test] + fn parses_cmdline_without_nul_terminators() -> Result<(), AndroidBootError> { + let mut bytes = image(b"kernel", b"ramdisk", b""); + let header = BootImageHeaderV2::mut_from_bytes(&mut bytes[..HEADER_SIZE]) + .map_err(|_| AndroidBootError::InvalidHeader)?; + header.cmdline.fill(b'x'); + header.extra_cmdline.fill(b'y'); + + let image = AndroidBootImage::parse(&mut Cursor::new(bytes))?; + let expected = format!( + "{}{}", + "x".repeat(CMDLINE_SIZE), + "y".repeat(EXTRA_CMDLINE_SIZE) + ); + assert_eq!(image.cmdline()?, expected); + assert!(CStr::from_bytes_until_nul(&image.cmdline).is_err()); + Ok(()) + } + #[test] fn rejects_non_utf8_cmdline() -> Result<(), AndroidBootError> { let bytes = image_with_cmdline(b"kernel", b"ramdisk", b"", &[0xff]); From 0ca2f4441ea8747eba4a51038630909400610758 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 27 Sep 2026 08:59:42 -0400 Subject: [PATCH 2/2] tree: Replace ExternalNoVerity with ExternalPath, keep ostree's redirect ostree gives each file a libcomposefs payload of `xx/.file` and sets the fsverity digest separately. The capi turned the payload into an ObjectID (dropping `.file`) or ignored it when a digest was set, so the redirect pointed at a file that doesn't exist and an ostree deployment on the Rust libcomposefs couldn't boot. ExternalPath carries an optional redirect as given plus an optional verity digest, which is how libcomposefs models it. RegularFile::external() builds it from the pair and keeps the usual composefs layout (redirect is the digest's object path) as External, so the capi, the EROFS reader and the dumpfile parser all normalize the same way. A digest without a payload now stays that way everywhere, as in C: no redirect is made up for it, and the dumpfile parser accepts it. composefs-info lists the redirect as the object path, like C's, so ostree images don't lose their objects there. In V2 images ExternalPath gets the same single null chunk index as External. Note Item::Regular's path in dumpfile_parse is now an Option, and a redirect with an interior NUL fails the capi conversion instead of being dropped. The new test builds an image like ostree does via the C API and pins its digest; `just test-capi` compares the same image against the C library. Generated-by: AI Signed-off-by: Colin Walters --- crates/composefs-boot/src/selabel.rs | 6 +- crates/composefs-capi/Cargo.toml | 1 + crates/composefs-capi/build.rs | 1 + crates/composefs-capi/src/convert.rs | 87 +++++---- crates/composefs-capi/src/image.rs | 9 +- crates/composefs-capi/src/tests.rs | 141 +++++++++++++++ .../tests/Containerfile.test-capi | 9 + crates/composefs-capi/tests/ostree-image.c | 129 ++++++++++++++ .../tests/test-capi-container.sh | 2 +- crates/composefs-ctl/src/composefs_info.rs | 77 +++++--- crates/composefs-ctl/src/lib.rs | 27 +-- crates/composefs-oci/src/delta.rs | 4 +- crates/composefs-ostree/src/commit.rs | 7 +- crates/composefs/src/dumpfile.rs | 75 ++++++-- crates/composefs/src/dumpfile_parse.rs | 20 ++- crates/composefs/src/erofs/reader.rs | 48 ++--- crates/composefs/src/erofs/writer.rs | 42 +++-- crates/composefs/src/fs.rs | 10 +- crates/composefs/src/tree.rs | 168 +++++++++++++++++- 19 files changed, 703 insertions(+), 160 deletions(-) create mode 100644 crates/composefs-capi/tests/ostree-image.c diff --git a/crates/composefs-boot/src/selabel.rs b/crates/composefs-boot/src/selabel.rs index a793802f..eaeeda1d 100644 --- a/crates/composefs-boot/src/selabel.rs +++ b/crates/composefs-boot/src/selabel.rs @@ -336,9 +336,9 @@ pub fn open_file( match dir.get_file_opt(filename.as_ref())? { Some(file) => match file { RegularFile::Inline(data) => Ok(Some(Box::new(Cursor::new(data.clone())))), - RegularFile::External(id, ..) | RegularFile::ExternalNoVerity(id, ..) => { - Ok(Some(Box::new(File::from(repo.open_object(id)?)))) - } + RegularFile::External(..) | RegularFile::ExternalPath { .. } => Ok(Some(Box::new( + File::from(repo.open_object(&file.repo_object_id()?)?), + ))), RegularFile::Sparse(..) => Ok(None), }, None => Ok(None), diff --git a/crates/composefs-capi/Cargo.toml b/crates/composefs-capi/Cargo.toml index 722ab703..816e00b3 100644 --- a/crates/composefs-capi/Cargo.toml +++ b/crates/composefs-capi/Cargo.toml @@ -20,6 +20,7 @@ rhel9 = ['composefs/rhel9'] composefs = { workspace = true } hex = { version = "0.4.0", default-features = false, features = ["std"] } libc = "0.2" +log = { version = "0.4", default-features = false } anyhow = "1" rustix = { version = "1", features = ["fs", "mm", "process", "mount"] } zerocopy = "0.8" diff --git a/crates/composefs-capi/build.rs b/crates/composefs-capi/build.rs index a399b0e2..6947e6c9 100644 --- a/crates/composefs-capi/build.rs +++ b/crates/composefs-capi/build.rs @@ -3,6 +3,7 @@ fn main() { cc::Build::new() .file("tests/test_lcfs.c") + .file("tests/ostree-image.c") .include("include/libcomposefs") .warnings(false) .compile("test_lcfs_c"); diff --git a/crates/composefs-capi/src/convert.rs b/crates/composefs-capi/src/convert.rs index ecb1a655..fb4b2e2e 100644 --- a/crates/composefs-capi/src/convert.rs +++ b/crates/composefs-capi/src/convert.rs @@ -3,6 +3,7 @@ use std::ffi::{CStr, CString, OsStr, OsString}; use std::os::unix::ffi::OsStrExt; use std::ptr; +use anyhow::Context; use zerocopy::{FromBytes, IntoBytes}; use composefs::fsverity::{FsVerityHashValue, Sha256HashValue}; @@ -134,30 +135,28 @@ fn ffi_node_to_leaf_content(node: &FfiNode) -> anyhow::Result 0 { - Ok(generic_tree::LeafContent::Regular(RegularFile::Sparse( - node.inode.st_size, - ))) - } else { + } else if node.inode.st_size == 0 { + // libcomposefs ignores the payload and digest of empty files Ok(generic_tree::LeafContent::Regular(RegularFile::Inline( Box::new([]), ))) + } else { + // Like libcomposefs, the payload (e.g. ostree's `xx/.file`) + // is the redirect as is, and the digest only goes into the metacopy + // xattr: they needn't be related. + let redirect = (!node.payload.is_null()).then(|| { + Box::from(OsStr::from_bytes( + unsafe { CStr::from_ptr(node.payload) }.to_bytes(), + )) + }); + let verity = node.digest_set.then(|| { + Sha256HashValue::read_from_bytes(&node.digest).expect("digest size mismatch") + }); + Ok(generic_tree::LeafContent::Regular(RegularFile::external( + redirect, + verity, + node.inode.st_size, + ))) } } t if t == libc::S_IFLNK => { @@ -187,7 +186,9 @@ fn ffi_node_to_leaf_content(node: &FfiNode) -> anyhow::Result) -> *mut FfiNode { +pub(crate) fn filesystem_to_ffi_tree( + fs: &tree::FileSystem, +) -> anyhow::Result<*mut FfiNode> { let mut root = Box::new(FfiNode::default()); stat_to_ffi(&fs.root.stat, &mut root); root.inode.st_mode |= libc::S_IFDIR; @@ -198,9 +199,14 @@ pub(crate) fn filesystem_to_ffi_tree(fs: &tree::FileSystem) -> let root_ptr = Box::into_raw(root); - fs_dir_to_ffi(&fs.root, &fs.leaves, &nlinks, root_ptr, &mut leaf_node_map); + if let Err(e) = fs_dir_to_ffi(&fs.root, &fs.leaves, &nlinks, root_ptr, &mut leaf_node_map) { + // SAFETY: root_ptr came from Box::into_raw above and every node + // created so far is attached to it, so this frees them all. + unsafe { crate::node::lcfs_node_unref(root_ptr) }; + return Err(e); + } - root_ptr + Ok(root_ptr) } fn fs_dir_to_ffi( @@ -209,7 +215,7 @@ fn fs_dir_to_ffi( nlinks: &[u32], parent: *mut FfiNode, leaf_node_map: &mut HashMap, -) { +) -> anyhow::Result<()> { for (name, inode) in dir.sorted_entries() { match inode { generic_tree::Inode::Directory(subdir) => { @@ -220,13 +226,15 @@ fn fs_dir_to_ffi( child.name = CString::new(name_bytes).map_or(ptr::null_mut(), CString::into_raw); child.parent = parent; + // Attach it before recursing, so that on an error the caller + // frees it with the rest of the tree. let child_ptr = Box::into_raw(child); - fs_dir_to_ffi(subdir, leaves, nlinks, child_ptr, leaf_node_map); unsafe { let mut children = (*parent).children_as_vec(); children.push(child_ptr); (*parent).children_put_back(children); } + fs_dir_to_ffi(subdir, leaves, nlinks, child_ptr, leaf_node_map)?; } generic_tree::Inode::Leaf(leaf_id, _) => { let leaf = &leaves[leaf_id.0]; @@ -251,7 +259,8 @@ fn fs_dir_to_ffi( let mut child = Box::new(FfiNode::default()); stat_to_ffi(&leaf.stat, &mut child); - leaf_content_to_ffi(&leaf.content, &mut child); + leaf_content_to_ffi(&leaf.content, &mut child) + .with_context(|| format!("Converting {name:?}"))?; let name_bytes = name.as_bytes(); child.name = CString::new(name_bytes).map_or(ptr::null_mut(), CString::into_raw); child.parent = parent; @@ -271,9 +280,13 @@ fn fs_dir_to_ffi( } } } + Ok(()) } -fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut FfiNode) { +fn leaf_content_to_ffi( + content: &tree::LeafContent, + node: &mut FfiNode, +) -> anyhow::Result<()> { match content { generic_tree::LeafContent::Regular(reg) => { node.inode.st_mode = (node.inode.st_mode & !libc::S_IFMT) | libc::S_IFREG; @@ -291,10 +304,21 @@ fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut let path = digest.to_object_pathname(); node.payload = CString::new(path).map_or(ptr::null_mut(), CString::into_raw); } - RegularFile::ExternalNoVerity(digest, size) => { + RegularFile::ExternalPath { + redirect, + verity, + size, + } => { node.inode.st_size = *size; - let path = digest.to_object_pathname(); - node.payload = CString::new(path).map_or(ptr::null_mut(), CString::into_raw); + if let Some(digest) = verity { + node.digest.copy_from_slice(digest.as_bytes()); + node.digest_set = true; + } + if let Some(redirect) = redirect { + node.payload = CString::new(redirect.as_bytes()) + .with_context(|| format!("Invalid redirect {redirect:?}"))? + .into_raw(); + } } RegularFile::Sparse(size) => { node.inode.st_size = *size; @@ -322,4 +346,5 @@ fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut node.inode.st_mode = (node.inode.st_mode & !libc::S_IFMT) | libc::S_IFSOCK; } } + Ok(()) } diff --git a/crates/composefs-capi/src/image.rs b/crates/composefs-capi/src/image.rs index 2e6d69e6..2302f98c 100644 --- a/crates/composefs-capi/src/image.rs +++ b/crates/composefs-capi/src/image.rs @@ -74,7 +74,14 @@ pub unsafe extern "C" fn lcfs_load_node_from_image_ext( } }; - let root = filesystem_to_ffi_tree(&fs); + let root = match filesystem_to_ffi_tree(&fs) { + Ok(root) => root, + Err(e) => { + log::debug!("Converting the loaded image for the C API: {e:#}"); + set_errno(libc::EINVAL); + return ptr::null_mut(); + } + }; // Apply toplevel_entries filter if specified if !options.is_null() { diff --git a/crates/composefs-capi/src/tests.rs b/crates/composefs-capi/src/tests.rs index c2b20f98..bb70e1b0 100644 --- a/crates/composefs-capi/src/tests.rs +++ b/crates/composefs-capi/src/tests.rs @@ -392,3 +392,144 @@ fn test_clone_deep_rewrites_hardlinks() { node::lcfs_node_unref(root); } } + +unsafe extern "C" { + fn write_ostree_image(fd: libc::c_int) -> libc::c_int; +} + +/// The fsverity digest of the image `tests/ostree-image.c` writes, as written +/// by the C libcomposefs. To regenerate it with the C library installed: +/// +/// ```text +/// cc -DOSTREE_IMAGE_MAIN -o ostree-image tests/ostree-image.c \ +/// $(pkg-config --cflags --libs composefs) +/// ./ostree-image > image && fsverity digest image +/// ``` +const OSTREE_IMAGE_C_DIGEST: &str = + "ed8a80347e9a3b0566dfefcd65509c47551746d9ae6c2dcfd0660ba4810c1013"; + +/// ostree gives each regular file a payload of `xx/.file` and sets +/// the fsverity digest separately; the image must be byte-identical to what +/// the C libcomposefs writes, or overlayfs can't find the file contents. +#[test] +fn test_ostree_image_matches_c() { + use composefs::fsverity::{FsVerityHashValue, Sha256HashValue, compute_verity}; + use composefs::tree::RegularFile; + use std::ffi::OsStr; + use std::io::Read; + use std::os::unix::ffi::OsStrExt; + use zerocopy::IntoBytes; + + let name = CString::new("ostree-image").unwrap(); + let fd = unsafe { libc::memfd_create(name.as_ptr(), 0) }; + assert!(fd >= 0, "memfd_create failed"); + let mut file = unsafe { std::fs::File::from_raw_fd(fd) }; + let ret = unsafe { write_ostree_image(file.as_raw_fd()) }; + assert_eq!(ret, 0, "write_ostree_image failed"); + file.rewind().unwrap(); + let mut image = Vec::new(); + file.read_to_end(&mut image).unwrap(); + + let fs = composefs::erofs::reader::erofs_to_filesystem::(&image).unwrap(); + let file_at = |path: &str| { + let (dir, name) = fs.root.split(OsStr::new(path)).unwrap(); + dir.get_file(name, &fs.leaves).unwrap() + }; + let expected_verity: Vec = (0..32u8) + .map(|i| i.wrapping_mul(7).wrapping_add(3)) + .collect(); + match file_at("/usr/bin/bash") { + RegularFile::ExternalPath { + redirect: Some(redirect), + verity: Some(verity), + size: 1432144, + } => { + assert_eq!( + redirect.as_bytes(), + b"8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file" + ); + assert_eq!(verity.as_bytes(), expected_verity); + } + other => panic!("unexpected /usr/bin/bash: {other:?}"), + } + match file_at("/usr/lib/libfoo.so.1") { + RegularFile::ExternalPath { + redirect: Some(redirect), + verity: None, + size: 8193, + } => assert_eq!( + redirect.as_bytes(), + b"e3/b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.file" + ), + other => panic!("unexpected /usr/lib/libfoo.so.1: {other:?}"), + } + match file_at("/usr/lib/verity-only") { + RegularFile::ExternalPath { + redirect: None, + verity: Some(verity), + size: 4097, + } => assert_eq!(verity.as_bytes(), expected_verity), + other => panic!("unexpected /usr/lib/verity-only: {other:?}"), + } + match file_at("/usr/lib/object") { + RegularFile::External(verity, 5000) => assert_eq!(verity.as_bytes(), expected_verity), + other => panic!("unexpected /usr/lib/object: {other:?}"), + } + + let digest: Sha256HashValue = compute_verity(&image); + assert_eq!(digest.to_hex(), OSTREE_IMAGE_C_DIGEST); +} + +/// A redirect with an interior NUL can't become a C payload, and must fail +/// the conversion instead of silently dropping the redirect. The entries +/// sorting before it (a subdirectory and a hardlinked pair, with xattrs) are +/// already converted by then, so this also exercises freeing a partial tree +/// (meant to be run under ASan or Miri too). +#[test] +fn test_redirect_with_nul_fails() { + use composefs::fsverity::Sha256HashValue; + use composefs::generic_tree::{Directory, Inode, LeafContent, Stat}; + use composefs::tree::{FileSystem, RegularFile}; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let stat = |mode| Stat { + st_mode: mode, + st_uid: 0, + st_gid: 0, + st_mtim_sec: 0, + st_mtim_nsec: 0, + xattrs: [( + Box::from(OsStr::new("user.test")), + Box::from(b"value".as_slice()), + )] + .into(), + }; + let inline = |data: &[u8]| LeafContent::Regular(RegularFile::Inline(Box::from(data))); + let mut fs = FileSystem::::new(stat(0o755)); + + let mut subdir = Directory::new(stat(0o755)); + let id = fs.push_leaf(stat(0o644), inline(b"in subdir")); + subdir.insert(OsStr::new("file"), Inode::leaf(id)); + fs.root + .insert(OsStr::new("a-dir"), Inode::Directory(Box::new(subdir))); + + let id = fs.push_leaf(stat(0o644), inline(b"hardlinked")); + fs.root.insert(OsStr::new("b-link1"), Inode::leaf(id)); + fs.root.insert(OsStr::new("b-link2"), Inode::leaf(id)); + + let id = fs.push_leaf( + stat(0o644), + LeafContent::Regular(RegularFile::external( + Some(Box::from(OsStr::from_bytes(b"8a/5d\0.file"))), + None, + 4096, + )), + ); + fs.root.insert(OsStr::new("z-bad"), Inode::leaf(id)); + + let err = crate::convert::filesystem_to_ffi_tree(&fs).unwrap_err(); + let err = format!("{err:#}"); + assert!(err.contains("Invalid redirect"), "{err}"); + assert!(err.contains("z-bad"), "{err}"); +} diff --git a/crates/composefs-capi/tests/Containerfile.test-capi b/crates/composefs-capi/tests/Containerfile.test-capi index e0c602e0..970bcc0b 100644 --- a/crates/composefs-capi/tests/Containerfile.test-capi +++ b/crates/composefs-capi/tests/Containerfile.test-capi @@ -5,6 +5,11 @@ COPY composefs-c /src/composefs-c COPY libcomposefs_capi.so /usr/lib64/ WORKDIR /src/composefs-c RUN meson setup build && meson compile -C build +# Write an ostree-style image with the C libcomposefs, to compare against ours +COPY ostree-image.c /src/ +RUN gcc -DOSTREE_IMAGE_MAIN -Ilibcomposefs -o /src/ostree-image /src/ostree-image.c \ + -Lbuild/libcomposefs -lcomposefs && \ + LD_LIBRARY_PATH=build/libcomposefs /src/ostree-image > /src/ostree-image-c.erofs RUN cp /usr/lib64/libcomposefs_capi.so build/libcomposefs/libcomposefs.so.1.4.0 && \ ln -sf libcomposefs.so.1.4.0 build/libcomposefs/libcomposefs.so.1 && \ ln -sf libcomposefs.so.1 build/libcomposefs/libcomposefs.so @@ -12,6 +17,10 @@ RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ ldd build/tools/mkcomposefs | grep composefs && \ tests/test-checksums.sh build/tools tests/assets \ "config.dump.gz config-with-hard-link.dump.gz special.dump bigfile.dump bigfile-xattr.dump special_v1.dump honggfuzz-long-symlink.dump honggfuzz-longlink-unterminated.dump honggfuzz-bigfile-with-acl.dump no-newline.dump honggfuzz-chardev-nonzero-size.dump longlink.dump honggfuzz-write-inode-data.dump" +RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ + /src/ostree-image > /src/ostree-image-rust.erofs && \ + cmp /src/ostree-image-c.erofs /src/ostree-image-rust.erofs && \ + echo "ostree-style image matches the C libcomposefs" RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ tests/test-units.sh build/tools && \ echo "=== All C tests passed with Rust libcomposefs ===" diff --git a/crates/composefs-capi/tests/ostree-image.c b/crates/composefs-capi/tests/ostree-image.c new file mode 100644 index 00000000..0e3472e9 --- /dev/null +++ b/crates/composefs-capi/tests/ostree-image.c @@ -0,0 +1,129 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR Apache-2.0 */ +/* + * Build an EROFS image the way ostree does (see ostree_repo_checkout_composefs() + * in ostree's src/libostree/ostree-repo-composefs.c): regular files have a + * payload of "xx/.file" pointing into the bare repo, with an fsverity + * digest only when verity is enabled, and empty files have no payload. + * + * This is linked into the composefs-capi tests, which pin the image's digest, + * and built standalone (with -DOSTREE_IMAGE_MAIN, writing the image to stdout) + * against both the C libcomposefs and ours to compare the output. + */ +#define _GNU_SOURCE + +#include "lcfs-writer.h" +#include +#include +#include +#include + +#define OSTREE_OBJ_A "8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file" +#define OSTREE_OBJ_B "e3/b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.file" +/* The composefs object path of the digest below */ +#define DIGEST_OBJ "03/0a11181f262d343b424950575e656c737a81888f969da4abb2b9c0c7ced5dc" + +static ssize_t write_fd_cb(void *file, void *buf, size_t count) +{ + return write(*(int *)file, buf, count); +} + +static struct lcfs_node_s *add_node(struct lcfs_node_s *parent, + const char *name, uint32_t mode, uint64_t size) +{ + struct lcfs_node_s *node = lcfs_node_new(); + if (node == NULL) + return NULL; + if (lcfs_node_add_child(parent, node, name) != 0) { + lcfs_node_unref(node); + return NULL; + } + lcfs_node_set_mode(node, mode); + lcfs_node_set_uid(node, 0); + lcfs_node_set_gid(node, 0); + lcfs_node_set_size(node, size); + return node; +} + +/* Returns 0 on success, or -1 with errno set. */ +int write_ostree_image(int fd) +{ + static const char selinux_bin[] = "system_u:object_r:bin_t:s0"; + static const char selinux_lib[] = "system_u:object_r:lib_t:s0"; + uint8_t digest[LCFS_DIGEST_SIZE]; + struct lcfs_node_s *root, *usr, *bin, *lib, *node; + int r = -1; + + for (size_t i = 0; i < sizeof(digest); i++) + digest[i] = (uint8_t)(i * 7 + 3); + + root = lcfs_node_new(); + if (root == NULL) + return -1; + lcfs_node_set_mode(root, S_IFDIR | 0755); + + if ((usr = add_node(root, "usr", S_IFDIR | 0755, 0)) == NULL) + goto out; + if ((bin = add_node(usr, "bin", S_IFDIR | 0755, 0)) == NULL) + goto out; + if ((lib = add_node(usr, "lib", S_IFDIR | 0755, 0)) == NULL) + goto out; + + /* A file with verity enabled: payload and digest are independent */ + if ((node = add_node(bin, "bash", S_IFREG | 0755, 1432144)) == NULL) + goto out; + if (lcfs_node_set_payload(node, OSTREE_OBJ_A) != 0) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + if (lcfs_node_set_xattr(node, "security.selinux", selinux_bin, + sizeof(selinux_bin)) != 0) + goto out; + + /* A file without verity: only the payload */ + if ((node = add_node(lib, "libfoo.so.1", S_IFREG | 0644, 8193)) == NULL) + goto out; + if (lcfs_node_set_payload(node, OSTREE_OBJ_B) != 0) + goto out; + if (lcfs_node_set_xattr(node, "security.selinux", selinux_lib, + sizeof(selinux_lib)) != 0) + goto out; + + /* Not from ostree: a digest without a payload, which gets no redirect */ + if ((node = add_node(lib, "verity-only", S_IFREG | 0644, 4097)) == NULL) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + + /* Not from ostree: the usual composefs layout, payload = digest's path */ + if ((node = add_node(lib, "object", S_IFREG | 0644, 5000)) == NULL) + goto out; + if (lcfs_node_set_payload(node, DIGEST_OBJ) != 0) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + + /* An empty file has no payload */ + if (add_node(lib, "empty", S_IFREG | 0644, 0) == NULL) + goto out; + + if ((node = add_node(bin, "sh", S_IFLNK | 0777, strlen("bash"))) == NULL) + goto out; + if (lcfs_node_set_payload(node, "bash") != 0) + goto out; + + struct lcfs_write_options_s options = { 0 }; + options.format = LCFS_FORMAT_EROFS; + options.version = 0; + options.max_version = 1; + options.file = &fd; + options.file_write_cb = write_fd_cb; + r = lcfs_write_to(root, &options); + +out: + lcfs_node_unref(root); + return r; +} + +#ifdef OSTREE_IMAGE_MAIN +int main(void) +{ + return write_ostree_image(STDOUT_FILENO) == 0 ? 0 : 1; +} +#endif diff --git a/crates/composefs-capi/tests/test-capi-container.sh b/crates/composefs-capi/tests/test-capi-container.sh index 7cdc3e84..472e71b7 100755 --- a/crates/composefs-capi/tests/test-capi-container.sh +++ b/crates/composefs-capi/tests/test-capi-container.sh @@ -14,7 +14,7 @@ cargo build --release -p composefs-capi --manifest-path="$REPO_DIR/Cargo.toml" BUILD_CTX=$(mktemp -d) trap 'rm -rf "$BUILD_CTX"' EXIT -cp "$REPO_DIR/target/release/libcomposefs_capi.so" "$BUILD_CTX/" +cp "$REPO_DIR/target/release/libcomposefs_capi.so" "$SCRIPT_DIR/ostree-image.c" "$BUILD_CTX/" if [ -n "$C_REPO" ]; then cp -a "$(cd "$C_REPO" && pwd)" "$BUILD_CTX/composefs-c" diff --git a/crates/composefs-ctl/src/composefs_info.rs b/crates/composefs-ctl/src/composefs_info.rs index e2872460..85a16045 100644 --- a/crates/composefs-ctl/src/composefs_info.rs +++ b/crates/composefs-ctl/src/composefs_info.rs @@ -18,8 +18,11 @@ //! filesystem doesn't support it, matching the C `lcfs_fd_get_fsverity()` //! behaviour. -use std::collections::HashSet; +use std::borrow::Cow; +use std::collections::{BTreeSet, HashSet}; +use std::ffi::{OsStr, OsString}; use std::io::Write; +use std::os::unix::ffi::OsStrExt; use std::path::Path; use std::{fs::File, io::Read, path::PathBuf}; @@ -31,7 +34,7 @@ use composefs::{ erofs::reader::erofs_to_filesystem, fsverity::{FsVerityHashValue, Sha256HashValue, measure_verity_with_fallback}, generic_tree::{Inode, LeafContent, LeafId}, - tree::{FileSystem, RegularFile}, + tree::FileSystem, }; /// Query information from composefs images. @@ -177,9 +180,9 @@ fn ls_print( match &leaf.content { LeafContent::Regular(regular) => { let is_hardlink = !seen_leaf_ids.insert(*leaf_id); - if !is_hardlink && let RegularFile::External(id, _) = regular { + if !is_hardlink && let Some(path) = regular.backing_path() { write!(out, "\t@ ")?; - print_escaped(out, id.to_object_pathname().as_bytes())?; + print_escaped(out, path.as_bytes())?; } } LeafContent::Symlink(target) => { @@ -239,17 +242,17 @@ fn cmd_dump(_filter: &[String], images: &[PathBuf]) -> Result<()> { Ok(()) } -/// Collect all external object IDs from a parsed filesystem. +/// Collect the backing paths of all external files in a parsed filesystem. /// -/// Iterates the leaves table directly — each `RegularFile::External` entry -/// is a unique content-addressed object. Because `erofs_to_filesystem` -/// deduplicates hard-linked inodes into a single leaf, each object appears -/// exactly once even if it is referenced by multiple paths. -fn collect_objects_from_fs(fs: &FileSystem) -> HashSet { +/// Like `composefs-info` from libcomposefs, these are the payloads (the +/// paths the overlay redirects point at, relative to the object store), so +/// ostree's `xx/.file` objects are listed as they are. Files +/// with only a verity digest have no backing path and aren't listed. +fn collect_objects_from_fs(fs: &FileSystem) -> BTreeSet { fs.leaves .iter() .filter_map(|leaf| match &leaf.content { - LeafContent::Regular(RegularFile::External(id, _)) => Some(id.clone()), + LeafContent::Regular(file) => file.backing_path().map(Cow::into_owned), _ => None, }) .collect() @@ -262,19 +265,25 @@ fn cmd_objects(images: &[PathBuf]) -> Result<()> { let fs = erofs_to_filesystem::(&image_data) .with_context(|| format!("Failed to parse image: {image_path:?}"))?; - let mut objects: Vec = collect_objects_from_fs(&fs).into_iter().collect(); - objects.sort_by_key(|id| id.to_hex()); - - for obj in objects { - println!("{}", obj.to_object_pathname()); + for obj in collect_objects_from_fs(&fs) { + println!("{}", obj.display()); } } Ok(()) } +/// Returns an object's path relative to the object store, without the +/// leading slashes a payload may have, so that joining it to the base +/// directory can't replace it (like C's `abs_to_rel_path()`). +fn object_relative_path(obj: &OsStr) -> &Path { + let bytes = obj.as_bytes(); + let start = bytes.iter().position(|&b| b != b'/').unwrap_or(bytes.len()); + Path::new(OsStr::from_bytes(&bytes[start..])) +} + /// List objects not present in basedir. fn cmd_missing_objects(basedir: &Path, images: &[PathBuf]) -> Result<()> { - let mut all_objects: HashSet = HashSet::new(); + let mut all_objects = BTreeSet::new(); for image_path in images { let image_data = read_image(image_path)?; @@ -283,15 +292,10 @@ fn cmd_missing_objects(basedir: &Path, images: &[PathBuf]) -> Result<()> { all_objects.extend(collect_objects_from_fs(&fs)); } - let mut missing: Vec = all_objects - .into_iter() - .filter(|obj| !basedir.join(obj.to_object_pathname()).exists()) - .collect(); - - missing.sort_by_key(|a| a.to_hex()); - - for obj in missing { - println!("{}", obj.to_object_pathname()); + for obj in all_objects { + if !basedir.join(object_relative_path(&obj)).exists() { + println!("{}", obj.display()); + } } Ok(()) @@ -316,3 +320,24 @@ fn read_image(path: &PathBuf) -> Result> { .with_context(|| format!("Failed to read image: {path:?}"))?; Ok(data) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_object_relative_path() { + let cases = [ + ("ab/cdef", "ab/cdef"), + ("8a/5d74.file", "8a/5d74.file"), + ("/8a/5d74.file", "8a/5d74.file"), + ("//8a/5d74.file", "8a/5d74.file"), + ("/", ""), + ]; + for (obj, expected) in cases { + let rel = object_relative_path(OsStr::new(obj)); + assert_eq!(rel, Path::new(expected), "{obj}"); + assert!(Path::new("/base").join(rel).starts_with("/base"), "{obj}"); + } + } +} diff --git a/crates/composefs-ctl/src/lib.rs b/crates/composefs-ctl/src/lib.rs index 62dba2a0..2043beb0 100644 --- a/crates/composefs-ctl/src/lib.rs +++ b/crates/composefs-ctl/src/lib.rs @@ -1652,28 +1652,17 @@ pub fn dump_files( } Inode::Leaf(leaf_id, _) => { - use composefs::generic_tree::LeafContent::*; - use composefs::tree::RegularFile::*; - if backing_path_only { let leaf = fs.leaf(*leaf_id); - match &leaf.content { - Regular(f) => match f { - Inline(..) | Sparse(..) => { - writeln!(&mut out, "{} inline", file_path.display())?; - } - External(id, _) | ExternalNoVerity(id, _) => { - writeln!( - &mut out, - "{} {}", - file_path.display(), - id.to_object_pathname() - )?; - } - }, - _ => { - writeln!(&mut out, "{} inline", file_path.display())?; + let backing_path = match &leaf.content { + composefs::generic_tree::LeafContent::Regular(f) => f.backing_path(), + _ => None, + }; + match backing_path { + Some(path) => { + writeln!(&mut out, "{} {}", file_path.display(), path.display())? } + None => writeln!(&mut out, "{} inline", file_path.display())?, } continue; diff --git a/crates/composefs-oci/src/delta.rs b/crates/composefs-oci/src/delta.rs index 9f894902..9dcfee36 100644 --- a/crates/composefs-oci/src/delta.rs +++ b/crates/composefs-oci/src/delta.rs @@ -86,11 +86,11 @@ impl DeltaDataSource for ComposeFsDataSource CurrentFile::Inline(Cursor::new(data.to_vec())), - RegularFile::External(id, _size) | RegularFile::ExternalNoVerity(id, _size) => { + RegularFile::External(..) | RegularFile::ExternalPath { .. } => { let fd = self .source .repo - .open_object(id) + .open_object(&file.repo_object_id()?) .with_context(|| format!("Opening source object for {}", path.display()))?; CurrentFile::External(File::from(fd)) } diff --git a/crates/composefs-ostree/src/commit.rs b/crates/composefs-ostree/src/commit.rs index dc09f1c5..c321a220 100644 --- a/crates/composefs-ostree/src/commit.rs +++ b/crates/composefs-ostree/src/commit.rs @@ -305,8 +305,9 @@ impl CommitWriter { match &leaf.content { LeafContent::Regular( - RegularFile::External(obj_id, size) | RegularFile::ExternalNoVerity(obj_id, size), + file @ (RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. }), ) if !should_inline_file::(*size as usize) => { + let obj_id = &file.repo_object_id()?; // Stream through the hasher without buffering the entire file. let mut hasher = Sha256::new(); hasher.update(&*regular_header); @@ -329,8 +330,8 @@ impl CommitWriter { let content: Option> = match &leaf.content { LeafContent::Regular(RegularFile::Inline(data)) => Some(data.to_vec()), LeafContent::Regular( - RegularFile::External(obj_id, _) | RegularFile::ExternalNoVerity(obj_id, _), - ) => Some(repo.read_object(obj_id)?), + file @ (RegularFile::External(..) | RegularFile::ExternalPath { .. }), + ) => Some(repo.read_object(&file.repo_object_id()?)?), LeafContent::Regular(RegularFile::Sparse(_)) => { bail!("Sparse files not supported in ostree commit") } diff --git a/crates/composefs/src/dumpfile.rs b/crates/composefs/src/dumpfile.rs index 8d18e43d..89d46e25 100644 --- a/crates/composefs/src/dumpfile.rs +++ b/crates/composefs/src/dumpfile.rs @@ -191,9 +191,7 @@ pub fn write_leaf( data, None, ), - LeafContent::Regular( - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size), - ) => write_entry( + LeafContent::Regular(RegularFile::External(id, size)) => write_entry( writer, path, stat, @@ -205,6 +203,22 @@ pub fn write_leaf( &[], Some(&id.to_hex()), ), + LeafContent::Regular(RegularFile::ExternalPath { + redirect, + verity, + size, + }) => write_entry( + writer, + path, + stat, + FileType::RegularFile, + *size, + nlink, + 0, + redirect.as_deref().unwrap_or_default(), + &[], + verity.as_ref().map(|id| id.to_hex()).as_deref(), + ), LeafContent::Regular(RegularFile::Sparse(size)) => write_entry( writer, path, @@ -487,14 +501,12 @@ pub fn add_entry_to_filesystem( .. } => { let stat = entry_to_stat(&entry)?; - let leaf_content = if let Some(digest) = fsverity_digest.as_ref() { - let object_id = ObjectID::from_hex(digest)?; - LeafContent::Regular(RegularFile::External(object_id, size)) - } else { - let object_id = ObjectID::from_object_pathname(path.as_os_str().as_bytes()) - .map_err(|e| anyhow::anyhow!("invalid object pathname: {e}"))?; - LeafContent::Regular(RegularFile::ExternalNoVerity(object_id, size)) - }; + let verity = fsverity_digest + .as_deref() + .map(ObjectID::from_hex) + .transpose()?; + let redirect = path.as_deref().map(|p| Box::from(p.as_os_str())); + let leaf_content = LeafContent::Regular(RegularFile::external(redirect, verity, size)); let id = push_leaf(fs, stat, leaf_content); Inode::leaf(id) } @@ -762,6 +774,47 @@ mod tests { Ok(()) } + /// External files keep their payload: only a payload that is the + /// digest's object path becomes `External`; anything else, like ostree's + /// `xx/.file` or no payload at all, is kept as `ExternalPath`. + #[test] + fn test_external_payload_round_trip() -> Result<()> { + const DIGEST: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const OBJECT: &str = "01/23456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const OSTREE: &str = + "8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file"; + let cases = [ + (OBJECT, DIGEST, "External"), + (OSTREE, DIGEST, "ExternalPath"), + (OSTREE, "-", "ExternalPath"), + // A digest without a payload, as libcomposefs dumps it + ("-", DIGEST, "ExternalPath"), + ]; + for (payload, digest, variant) in cases { + let dumpfile = format!( + "/ 0 40755 2 0 0 0 0.0 - - -\n/f 4097 100644 1 0 0 0 0.0 {payload} - {digest}\n" + ); + let fs = dumpfile_to_filesystem::(&dumpfile)?; + let Some(Inode::Leaf(id, _)) = fs.root.lookup(OsStr::new("f")) else { + panic!("expected a leaf for {payload}"); + }; + let LeafContent::Regular(file) = &fs.leaf(*id).content else { + panic!("expected a regular file for {payload}"); + }; + let found = match file { + RegularFile::External(..) => "External", + RegularFile::ExternalPath { .. } => "ExternalPath", + other => panic!("unexpected {other:?} for {payload}"), + }; + assert_eq!(found, variant, "{payload} {digest}"); + + let mut out = Vec::new(); + write_dumpfile(&mut out, &fs)?; + assert_eq!(std::str::from_utf8(&out).unwrap(), dumpfile); + } + Ok(()) + } + /// Verify that xattrs with empty values and with a value of "-" /// both survive a round-trip. Previously `write_escaped` used /// the "-" sentinel for empty bytes, which the xattr parser does diff --git a/crates/composefs/src/dumpfile_parse.rs b/crates/composefs/src/dumpfile_parse.rs index 075efc8c..2b14bc2b 100644 --- a/crates/composefs/src/dumpfile_parse.rs +++ b/crates/composefs/src/dumpfile_parse.rs @@ -94,8 +94,9 @@ pub enum Item<'p> { size: u64, /// Number of links nlink: u32, - /// The backing store path - path: Cow<'p, Path>, + /// The backing store path; `None` for a file that only has an + /// fsverity digest, which overlayfs looks up by its own path + path: Option>, /// The fsverity digest fsverity_digest: Option, }, @@ -436,21 +437,22 @@ impl<'p> Entry<'p> { match ty { FileType::RegularFile => { Self::check_rdev(rdev)?; - if let Some(path) = payload.as_ref() { - let path = unescape_to_path(path)?; + if payload.is_some() || fsverity_digest.is_some() { + // libcomposefs accepts a digest without a payload, + // and writes it as a metacopy without a redirect. + if payload.is_none() && content.is_some() { + anyhow::bail!("Inline file cannot have fsverity digest"); + } Item::Regular { size, nlink, - path, + path: payload.map(unescape_to_path).transpose()?, fsverity_digest: fsverity_digest.map(ToOwned::to_owned), } } else { // A dumpfile entry with no backing path or payload is treated as an empty file let content = content.unwrap_or_default(); let content = unescape_limited(content, MAX_INLINE_CONTENT)?; - if fsverity_digest.is_some() { - anyhow::bail!("Inline file cannot have fsverity digest"); - } Item::RegularInline { nlink, size, @@ -560,7 +562,7 @@ impl Item<'_> { pub(crate) fn payload(&self) -> Option<&Path> { match self { - Item::Regular { path, .. } => Some(path), + Item::Regular { path, .. } => path.as_deref(), Item::Symlink { target, .. } => Some(target), Item::Hardlink { target } => Some(target), _ => None, diff --git a/crates/composefs/src/erofs/reader.rs b/crates/composefs/src/erofs/reader.rs index 874e93d0..6ba79633 100644 --- a/crates/composefs/src/erofs/reader.rs +++ b/crates/composefs/src/erofs/reader.rs @@ -1590,36 +1590,32 @@ fn extract_metacopy_digest( Ok(None) } -/// Try to extract the object ID from a redirect xattr (`trusted.overlay.redirect`). +/// Extract the path from a redirect xattr (`trusted.overlay.redirect`). /// -/// The redirect value is a path like `/55/90e94b...` from which we parse -/// the object ID. Returns `None` if no redirect xattr is present. -fn extract_redirect_object_id( - img: &Image, - inode: &InodeType, -) -> anyhow::Result> { +/// The redirect value is a path like `/55/90e94b...`; this returns it without +/// the leading `/`, as the inverse of what the writer does. Returns `None` if +/// no redirect xattr is present. +fn extract_redirect(img: &Image, inode: &InodeType) -> anyhow::Result>> { let Some(xattrs_section) = inode.xattrs()? else { return Ok(None); }; for id in xattrs_section.shared()? { let xattr = img.shared_xattr(id.get())?; - if let Some(obj) = check_redirect_xattr(xattr)? { - return Ok(Some(obj)); + if let Some(path) = check_redirect_xattr(xattr)? { + return Ok(Some(path)); } } for xattr in xattrs_section.local()? { let xattr = xattr?; - if let Some(obj) = check_redirect_xattr(xattr)? { - return Ok(Some(obj)); + if let Some(path) = check_redirect_xattr(xattr)? { + return Ok(Some(path)); } } Ok(None) } -fn check_redirect_xattr( - xattr: &XAttr, -) -> anyhow::Result> { +fn check_redirect_xattr(xattr: &XAttr) -> anyhow::Result>> { if xattr.header.name_index != 4 { return Ok(None); } @@ -1628,10 +1624,7 @@ fn check_redirect_xattr( } let value = xattr.value()?; let path = value.strip_prefix(b"/").unwrap_or(value); - match ObjectID::from_object_pathname(path) { - Ok(id) => Ok(Some(id)), - Err(_) => Ok(None), - } + Ok(Some(Box::from(OsStr::from_bytes(path)))) } /// Check if a single xattr is a valid overlay.metacopy and return the digest. @@ -1882,19 +1875,12 @@ fn populate_directory( } else { match file_type { S_IFREG => { - if let Some(digest) = - extract_metacopy_digest::(img, &child_inode)? - { - tree::LeafContent::Regular(tree::RegularFile::External( - digest, - child_inode.size(), - )) - } else if let Some(id) = - extract_redirect_object_id::(img, &child_inode)? - { - tree::LeafContent::Regular(tree::RegularFile::ExternalNoVerity( - id, - child_inode.size(), + let verity = extract_metacopy_digest::(img, &child_inode)?; + let redirect = extract_redirect(img, &child_inode)?; + let size = child_inode.size(); + if verity.is_some() || redirect.is_some() { + tree::LeafContent::Regular(tree::RegularFile::external( + redirect, verity, size, )) } else if child_inode.data_layout()? == DataLayout::ChunkBased { tree::LeafContent::Regular(tree::RegularFile::Sparse( diff --git a/crates/composefs/src/erofs/writer.rs b/crates/composefs/src/erofs/writer.rs index 0b0de92c..175c8a08 100644 --- a/crates/composefs/src/erofs/writer.rs +++ b/crates/composefs/src/erofs/writer.rs @@ -706,7 +706,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(.., size) - | tree::RegularFile::ExternalNoVerity(.., size) + | tree::RegularFile::ExternalPath { size, .. } | tree::RegularFile::Sparse(size), ) => { let chunk_format = match version.epoch() { @@ -757,7 +757,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..), ) => { let n_chunks = self.inline_tail_size / 4; @@ -794,7 +794,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..), ) => { const LCFS_MAX_NONINLINE_CHUNKS: usize = 1024; @@ -1178,17 +1178,29 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { self.version, ); } else if let InodeContent::Leaf(Leaf { - content: tree::LeafContent::Regular(tree::RegularFile::ExternalNoVerity(id, ..)), + content: + tree::LeafContent::Regular(tree::RegularFile::ExternalPath { + redirect, verity, .. + }), .. }) = content { - xattrs.add(format::XATTR_OVERLAY_METACOPY, b"", self.version); - let redirect = format!("/{}", id.to_object_pathname()); - xattrs.add( - format::XATTR_OVERLAY_REDIRECT, - redirect.as_bytes(), - self.version, - ); + // This matches libcomposefs, which writes the redirect as "/" + payload. + match verity { + Some(id) => { + let metacopy = OverlayMetacopy::new(id); + xattrs.add( + format::XATTR_OVERLAY_METACOPY, + metacopy.as_bytes(), + self.version, + ); + } + None => xattrs.add(format::XATTR_OVERLAY_METACOPY, b"", self.version), + } + if let Some(redirect) = redirect { + let redirect = [b"/", redirect.as_bytes()].concat(); + xattrs.add(format::XATTR_OVERLAY_REDIRECT, &redirect, self.version); + } } else if let InodeContent::Leaf(Leaf { content: tree::LeafContent::Regular(tree::RegularFile::Sparse(..)), .. @@ -1263,7 +1275,7 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(.., size) - | tree::RegularFile::ExternalNoVerity(.., size) + | tree::RegularFile::ExternalPath { size, .. } | tree::RegularFile::Sparse(size), ) if *size > 0 => { let chunk_count = compute_chunk_count(*size); @@ -1274,7 +1286,9 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { } else { match &leaf.content { tree::LeafContent::Regular(tree::RegularFile::Inline(data)) => (0, data.len()), - tree::LeafContent::Regular(tree::RegularFile::External(..)) => { + tree::LeafContent::Regular( + tree::RegularFile::External(..) | tree::RegularFile::ExternalPath { .. }, + ) => { (0, 4) // single null chunk index } _ => (0, 0), @@ -2056,7 +2070,7 @@ fn fixup_epoch1_data_blocks( leaf.content, tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..) ) ), diff --git a/crates/composefs/src/fs.rs b/crates/composefs/src/fs.rs index 8f882eeb..dccad199 100644 --- a/crates/composefs/src/fs.rs +++ b/crates/composefs/src/fs.rs @@ -308,9 +308,10 @@ fn write_leaf( set_file_contents(dirfd, name, &leaf.stat, data)? } LeafContent::Regular( - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size), + file @ (RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. }), ) => { - let object = repo.open_object(id)?; + let id = file.repo_object_id()?; + let object = repo.open_object(&id)?; // TODO: make this better. At least needs to be EINTR-safe. Could even do reflink in some cases. // Regardless we shouldn't read the whole file into memory. let size = (*size).try_into().context("size overflow")?; @@ -694,10 +695,11 @@ pub fn read_file( ) -> Result> { match file { RegularFile::Inline(data) => Ok(data.clone()), - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size) => { + RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. } => { + let id = file.repo_object_id()?; let capacity: usize = (*size).try_into().context("file too large for memory")?; let mut data = Vec::with_capacity(capacity); - std::fs::File::from(repo.open_object(id)?).read_to_end(&mut data)?; + std::fs::File::from(repo.open_object(&id)?).read_to_end(&mut data)?; ensure!( *size == data.len() as u64, "File content doesn't have the expected length" diff --git a/crates/composefs/src/tree.rs b/crates/composefs/src/tree.rs index 190c3f85..1d10d2ae 100644 --- a/crates/composefs/src/tree.rs +++ b/crates/composefs/src/tree.rs @@ -2,6 +2,10 @@ //! of inlining small files, and having an external fsverity reference for //! larger ones. +use std::borrow::Cow; +use std::ffi::OsStr; +use std::os::unix::ffi::OsStrExt; + use crate::fsverity::FsVerityHashValue; pub use crate::generic_tree::{self, ImageError, Stat}; @@ -19,10 +23,23 @@ pub enum RegularFile { /// The tuple contains (fsverity hash, file size in bytes). /// The fsverity digest is embedded in the overlay metacopy xattr. External(ObjectID, u64), - /// Like `External`, but without embedding the fsverity digest in the - /// overlay metacopy xattr. Used by the C API when the caller set a - /// content-address payload but did not explicitly set a verified digest. - ExternalNoVerity(ObjectID, u64), + /// File stored externally at an explicit path, as libcomposefs models it. + /// + /// Unlike `External`, the overlay redirect isn't derived from a digest. + /// This is what the C API produces for a node with a payload, e.g. ostree's + /// `xx/.file` objects. Build it with [`RegularFile::external`], + /// which keeps the cases `External` and `Sparse` cover out of it. + ExternalPath { + /// The backing file's path relative to the object store root, written + /// as `/` in the overlay redirect xattr. Without one, no + /// redirect is written and overlayfs looks the file up by its own path + /// in the data layers. + redirect: Option>, + /// The fsverity digest to embed in the overlay metacopy xattr, if any. + verity: Option, + /// The file size in bytes. + size: u64, + }, /// File with declared size but no content or external reference. /// Produces ChunkBased layout with null chunk indices. Sparse(u64), @@ -33,7 +50,72 @@ impl RegularFile { pub fn file_size(&self) -> u64 { match self { Self::Inline(data) => data.len() as u64, - Self::External(_, size) | Self::ExternalNoVerity(_, size) | Self::Sparse(size) => *size, + Self::External(_, size) | Self::ExternalPath { size, .. } | Self::Sparse(size) => *size, + } + } + + /// Builds an external file from an overlay redirect and verity digest, + /// the way libcomposefs stores them: independently of each other. + /// + /// An empty redirect counts as none, as in libcomposefs. The result is + /// `External` when the redirect is the digest's object path (the usual + /// composefs layout), `Sparse` when there is neither, and `ExternalPath` + /// otherwise. + pub fn external(redirect: Option>, verity: Option, size: u64) -> Self { + let redirect = redirect.filter(|r| !r.is_empty()); + match (redirect, verity) { + (None, None) => Self::Sparse(size), + (Some(redirect), Some(id)) + if redirect.as_bytes() == id.to_object_pathname().as_bytes() => + { + Self::External(id, size) + } + (redirect, verity) => Self::ExternalPath { + redirect, + verity, + size, + }, + } + } + + /// Returns the path of an external file's backing file relative to the + /// object store root, which the overlay redirect xattr points at: the + /// object path for `External`, the redirect for `ExternalPath`. + /// + /// This is the libcomposefs payload, which `composefs-info` lists. + pub fn backing_path(&self) -> Option> { + match self { + Self::External(id, _) => Some(Cow::Owned(id.to_object_pathname().into())), + Self::ExternalPath { redirect, .. } => redirect.as_deref().map(Cow::Borrowed), + Self::Inline(_) | Self::Sparse(_) => None, + } + } + + /// Returns the object backing an external file in a composefs repository. + /// + /// For `ExternalPath`, this is the verity digest if set, otherwise the + /// redirect parsed as an object pathname. Fails for inline and sparse + /// files, and for an `ExternalPath` that names no object this way (like + /// ostree's `xx/.file` without a verity digest). + pub fn repo_object_id(&self) -> anyhow::Result { + match self { + Self::Inline(_) | Self::Sparse(_) => anyhow::bail!("Not an external file"), + Self::External(id, _) => Ok(id.clone()), + Self::ExternalPath { + verity: Some(id), .. + } => Ok(id.clone()), + Self::ExternalPath { + redirect: Some(redirect), + verity: None, + .. + } => ObjectID::from_object_pathname(redirect.as_bytes()).map_err(|e| { + anyhow::anyhow!("External file path {redirect:?} is not an object: {e}") + }), + Self::ExternalPath { + redirect: None, + verity: None, + .. + } => anyhow::bail!("External file has neither a path nor a verity digest"), } } } @@ -139,4 +221,80 @@ mod tests { .unwrap(); assert_eq!(retrieved_subdir_opt.stat.st_mtim_sec, 20); } + + #[test] + fn test_external() { + let id = Sha256HashValue::from_hex( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ) + .unwrap(); + let other = Sha256HashValue::from_hex( + "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210", + ) + .unwrap(); + let object_path = id.to_object_pathname(); + const OSTREE: &str = "8a/5d74.file"; + // (redirect, verity, variant, repo_object_id, backing_path) + let cases = [ + ( + Some(object_path.as_str()), + Some(&id), + "External", + Some(&id), + Some(object_path.as_str()), + ), + // The verity digest names the repository object, whatever the redirect + ( + Some(OSTREE), + Some(&other), + "ExternalPath", + Some(&other), + Some(OSTREE), + ), + ( + Some(object_path.as_str()), + Some(&other), + "ExternalPath", + Some(&other), + Some(object_path.as_str()), + ), + (None, Some(&id), "ExternalPath", Some(&id), None), + (Some(""), Some(&id), "ExternalPath", Some(&id), None), + ( + Some(object_path.as_str()), + None, + "ExternalPath", + Some(&id), + Some(object_path.as_str()), + ), + // ostree's payload without verity names no repository object + (Some(OSTREE), None, "ExternalPath", None, Some(OSTREE)), + (None, None, "Sparse", None, None), + (Some(""), None, "Sparse", None, None), + ]; + for (redirect, verity, variant, object, backing_path) in cases { + let file = RegularFile::external( + redirect.map(|r| Box::from(OsStr::new(r))), + verity.cloned(), + 4096, + ); + let found = match &file { + RegularFile::External(..) => "External", + RegularFile::ExternalPath { .. } => "ExternalPath", + RegularFile::Sparse(..) => "Sparse", + RegularFile::Inline(..) => "Inline", + }; + assert_eq!(found, variant, "{redirect:?} {verity:?}"); + assert_eq!(file.file_size(), 4096); + assert_eq!(file.repo_object_id().ok().as_ref(), object, "{file:?}"); + assert_eq!( + file.backing_path().as_deref(), + backing_path.map(OsStr::new), + "{file:?}" + ); + } + let inline = RegularFile::::Inline(Box::new([1])); + assert!(inline.repo_object_id().is_err()); + assert_eq!(inline.backing_path(), None); + } }