From 32fb6689585475e42709c35d61a5a1ce18c89ce0 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Tue, 29 Sep 2026 11:24:23 +0200 Subject: [PATCH 1/4] 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 b83cec6105bc49dc3e19e2c5736f248615495c25 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 27 Sep 2026 08:59:42 -0400 Subject: [PATCH 2/4] 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); + } } From 9d74453e13a92d3f0fd864f6a7856efddcd2065b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 25 Sep 2026 11:06:01 -0400 Subject: [PATCH 3/4] packaging: Let pack.sh vendor with plain cargo pack.sh needs cargo-vendor-filterer, which isn't packaged anywhere and gets installed unpinned with `cargo install`. `PACK_VENDOR=cargo` uses `cargo vendor` instead, for builds that only need a working RPM and would rather not fetch and build another tool; the vendor tarball is bigger since it keeps every platform's crates. Prep for building RPMs in the bootc revdep test. Generated-by: AI Signed-off-by: Colin Walters --- contrib/packaging/pack.sh | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/contrib/packaging/pack.sh b/contrib/packaging/pack.sh index 2d7f0193..e1f70663 100755 --- a/contrib/packaging/pack.sh +++ b/contrib/packaging/pack.sh @@ -35,12 +35,33 @@ echo "Version: ${VERSION}" # Source tarball from git git archive --format=tar --prefix="${PREFIX}" -o "${TAR}" HEAD -# Vendor tarball via cargo-vendor-filterer -VENDOR_CONFIG=$(cargo vendor-filterer --prefix=vendor --format=tar.zstd "${VENDORTAR}") - -# Fix the vendor config to use a relative "vendor" directory TMPDIR=$(mktemp -d -p target) trap 'rm -rf "${TMPDIR}"' EXIT + +# Vendor tarball via cargo-vendor-filterer, or with PACK_VENDOR=cargo via +# plain `cargo vendor` (larger, as it keeps all platforms, but needs no +# extra tools). +case "${PACK_VENDOR:-filterer}" in + filterer) + VENDOR_CONFIG=$(cargo vendor-filterer --prefix=vendor --format=tar.zstd "${VENDORTAR}") + ;; + cargo) + VENDOR_CONFIG=$(cargo vendor "${TMPDIR}/vendor") + tar -C "${TMPDIR}" --zstd -cf "${VENDORTAR}" vendor + rm -rf "${TMPDIR}/vendor" + ;; + *) + echo "error: unknown PACK_VENDOR=${PACK_VENDOR}" >&2 + exit 1 + ;; +esac + +if test -z "${VENDOR_CONFIG}"; then + echo "error: vendoring printed no source replacement config" >&2 + exit 1 +fi + +# Fix the vendor config to use a relative "vendor" directory echo "${VENDOR_CONFIG}" | sed 's|^directory = ".*"|directory = "vendor"|' > "${TMPDIR}/vendor-config.toml" # Embed .cargo/vendor-config.toml into the source tarball From ffb38b15501d464d7ee6833d8f609edb18160a3c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 25 Sep 2026 11:06:01 -0400 Subject: [PATCH 4/4] bootc: Test ostree against the Rust libcomposefs ostree is the main C consumer of libcomposefs, and nothing tests it against our implementation yet: the capi job only runs the C library's own test suite. For composefs-rs to replace the C composefs package, ostree's composefs support (writing images at deploy time, mounting them from ostree-prepare-root) has to work on the Rust library. The new `just bootc/test-ostree` builds the composefs RPMs from this checkout with pack.sh and composefs.spec (vendoring with plain cargo) in a CentOS Stream 10 buildroot, and adds them to the packages bootc installs into its test image. bootc installs those from a local repository that takes priority over the distribution's, so they replace the C composefs. It checks that libostree loads our libcomposefs, and that the initramfs has the same library, then runs bootc's readonly and upgrade-with-reboot tests with the ostree backend. The revdep workflow runs it as a second matrix entry (still behind the workflow's temporary `if: never()`). Generated-by: AI Signed-off-by: Colin Walters --- .github/workflows/bootc-revdep.yml | 13 ++++- bootc/Justfile | 94 ++++++++++++++++++++++++++---- bootc/build-composefs-rpms | 39 +++++++++++++ 3 files changed, 134 insertions(+), 12 deletions(-) create mode 100755 bootc/build-composefs-rpms diff --git a/.github/workflows/bootc-revdep.yml b/.github/workflows/bootc-revdep.yml index 8ea1645e..5d806e96 100644 --- a/.github/workflows/bootc-revdep.yml +++ b/.github/workflows/bootc-revdep.yml @@ -21,10 +21,19 @@ permissions: jobs: bootc-test: - name: Build and test bootc with local composefs-rs + name: bootc with local composefs-rs (${{ matrix.name }}) if: never() runs-on: ubuntu-24.04 timeout-minutes: 120 + strategy: + fail-fast: false + matrix: + include: + - name: composefs backend + recipe: test + # ostree's composefs support running on our libcomposefs + - name: ostree backend + recipe: test-ostree steps: - name: Checkout repository @@ -44,4 +53,4 @@ jobs: echo "$HOME/.local/bin" >> $GITHUB_PATH - name: Build and test bootc with local composefs-rs - run: just bootc/test + run: just bootc/${{ matrix.recipe }} diff --git a/bootc/Justfile b/bootc/Justfile index a5737284..dc31f782 100644 --- a/bootc/Justfile +++ b/bootc/Justfile @@ -17,8 +17,17 @@ export COMPOSEFS_BOOTC_REF := env("COMPOSEFS_BOOTC_REF", "main") # Remote repository for bootc export COMPOSEFS_BOOTC_REPO := env("COMPOSEFS_BOOTC_REPO", "https://github.com/bootc-dev/bootc") +# Buildroot image for `package-composefs`; its distribution must match +# bootc's base image (CentOS Stream 10 by default). +export COMPOSEFS_BOOTC_BUILDROOT := env("COMPOSEFS_BOOTC_BUILDROOT", "quay.io/centos/centos:stream10") + # Internal: absolute path to the composefs-rs checkout (parent of this Justfile) export _COMPOSEFS_SRC := canonicalize(source_directory() + "/..") +# Internal: scratch clone and output directory for `package-composefs` +_composefs_rpm_src := COMPOSEFS_BOOTC_PATH + "/target/composefs-rpm-src" +_composefs_rpms := COMPOSEFS_BOOTC_PATH + "/target/composefs-packages" +# Internal: bootc's default output image name +_bootc_image := env("BOOTC_image", "localhost/bootc") # Clone or update bootc repository clone: @@ -47,19 +56,10 @@ clone: # the bind-mounted local source instead of fetching from git. # # Errors if the composefs-rs tree has uncommitted changes. -patch: clone +patch: clone _require-clean #!/bin/bash set -euo pipefail - # Require a clean composefs-rs working tree so we test a real commit. - # Only tracked files matter; untracked files are allowed. - # git diff HEAD already excludes untracked files. - if ! git -C "$_COMPOSEFS_SRC" diff --quiet HEAD 2>/dev/null; then - echo "error: composefs-rs has uncommitted changes — commit or stash first" >&2 - git -C "$_COMPOSEFS_SRC" diff --stat HEAD >&2 - exit 1 - fi - cfs_path="$_COMPOSEFS_SRC/crates/composefs-ctl" cd "$COMPOSEFS_BOOTC_PATH" @@ -92,6 +92,79 @@ patch: clone sed -i "s/^# Patched by composefs-rs.*/# Patched by composefs-rs at ${_rev}/" Cargo.toml echo "bootc patched for composefs-rs at ${_rev}" +# Require a clean composefs-rs working tree so we test a real commit. +# Only tracked files matter; untracked files are allowed. +_require-clean: + #!/bin/bash + set -euo pipefail + # git diff HEAD already excludes untracked files. + if ! git -C "$_COMPOSEFS_SRC" diff --quiet HEAD 2>/dev/null; then + echo "error: composefs-rs has uncommitted changes — commit or stash first" >&2 + git -C "$_COMPOSEFS_SRC" diff --stat HEAD >&2 + exit 1 + fi + +# Build the composefs RPMs from the HEAD commit of this checkout, in a +# buildroot matching bootc's base image. +package-composefs: clone _require-clean + #!/bin/bash + set -euo pipefail + rm -rf "{{_composefs_rpm_src}}" "{{_composefs_rpms}}" + mkdir -p "{{_composefs_rpms}}" + # A scratch clone, so this also works from a git worktree and + # leaves the checkout's target/ alone. + git clone -q "$_COMPOSEFS_SRC" "{{_composefs_rpm_src}}" + podman run --rm --security-opt=label=disable \ + -v "{{_composefs_rpm_src}}:/src" -v "{{_composefs_rpms}}:/out" \ + "$COMPOSEFS_BOOTC_BUILDROOT" /src/bootc/build-composefs-rpms /src /out + ls -l "{{_composefs_rpms}}" + +# Build a bootc image (ostree backend) whose composefs packages come from +# this checkout, replacing the distribution's C composefs. ostree then +# generates and mounts composefs images with the Rust libcomposefs. +build-ostree: patch package-composefs + #!/bin/bash + set -euo pipefail + cd "$COMPOSEFS_BOOTC_PATH" + export BOOTC_variant=ostree + # bootc's `package` recreates target/packages, so add ours after it. + # bootc installs every RPM there from a local repository that takes + # priority over the distribution's. + just package + cp -v "{{_composefs_rpms}}"/*.rpm target/packages/ + BOOTC_SKIP_PACKAGE=1 just build + # Check that the libcomposefs ostree loads is ours, in the image and + # in the initramfs (where ostree-prepare-root mounts the composefs). + podman run --rm --security-opt=label=disable \ + -v "{{_composefs_rpms}}:/run/composefs-packages:ro" "{{_bootc_image}}" bash -c ' + set -euo pipefail + lib=$(ldd /usr/lib64/libostree-1.so.1 | awk "/libcomposefs\\.so/ { print \$3 }") + lib=$(readlink -f "$lib") + owner=$(rpm -qf "$lib") + expected=$(rpm -qp /run/composefs-packages/composefs-libs-[0-9]*.rpm) + echo "libostree uses $lib from $owner" + if [ "$owner" != "$expected" ]; then + echo "error: expected libcomposefs from $expected" >&2 + exit 1 + fi + initramfs=$(ls /usr/lib/modules/*/initramfs.img) + tmp=$(mktemp -d) + (cd "$tmp" && lsinitrd --unpack "$initramfs" "${lib#/}" 2>/dev/null) + if ! cmp "$lib" "$tmp/$lib"; then + echo "error: the initramfs has a different $lib" >&2 + exit 1 + fi + echo "initramfs has the same $lib"' + +# Run bootc's ostree backend tests on the image from `build-ostree`: a +# bootc install and an upgrade with a reboot, so ostree writes composefs +# images and ostree-prepare-root mounts them through the Rust libcomposefs. +test-ostree *plans="readonly image-upgrade-reboot": build-ostree + #!/bin/bash + set -euo pipefail + cd "$COMPOSEFS_BOOTC_PATH" + BOOTC_variant=ostree BOOTC_SKIP_PACKAGE=1 just test-tmt {{plans}} + # Build sealed bootc image using local composefs-rs # The path dependency is auto-detected and bind-mounted by bootc's Justfile build: patch @@ -165,6 +238,7 @@ config: just bootc/test # Default: systemd + ext4 + uki + sealed just bootc/test grub ext4 bls unsealed # grub + ext4 + BLS (unsealed) just bootc/test systemd btrfs uki sealed # systemd-boot + btrfs + UKI (sealed) + just bootc/test-ostree # ostree backend with this libcomposefs Example Usage: just bootc/build # Clone main, patch, and build diff --git a/bootc/build-composefs-rpms b/bootc/build-composefs-rpms new file mode 100755 index 00000000..4b9e2aaa --- /dev/null +++ b/bootc/build-composefs-rpms @@ -0,0 +1,39 @@ +#!/bin/bash +# Build the composefs RPMs (libcomposefs, mkcomposefs, mount.composefs, ...) +# the same way Packit does: pack.sh generates the source and vendor +# tarballs, then rpmbuild builds contrib/packaging/composefs.spec. +# +# Runs inside a buildroot container matching the bootc base image (see +# `package-composefs` in the Justfile) on a scratch clone of this +# repository, so the resulting packages can replace the distribution's +# C composefs in the bootc test image. +# +# Usage: build-composefs-rpms SRC OUTDIR +set -xeuo pipefail + +src=$1 +out=$2 + +dnf -y install cargo rust gcc git make openssl-devel pkgconf-pkg-config \ + rpm-build zlib-devel zstd +git config --global --add safe.directory "$src" +cd "$src" + +# Plain `cargo vendor` with the distribution's cargo, rather than +# installing cargo-vendor-filterer as the release workflow does. +PACK_VENDOR=cargo contrib/packaging/pack.sh + +# Man pages need pandoc, and %autochangelog rpmautospec, neither of +# which CentOS Stream ships; the changelog doesn't matter here, and +# neither do debuginfo packages (the spec's release build has no +# debug info to split out). +topdir=$(mktemp -d) +# Offline, so the build must use the vendored crates, as in Koji. +CARGO_NET_OFFLINE=true rpmbuild -bb --without man \ + --define "autochangelog %{nil}" \ + --define "debug_package %{nil}" \ + --define "_topdir ${topdir}" \ + --define "_sourcedir ${src}/target" \ + target/composefs.spec + +cp "${topdir}"/RPMS/*/*.rpm "$out"/