Skip to content

Commit

Permalink
tar/export: Don't create hardlinks to zero sized files
Browse files Browse the repository at this point in the history
This is an echo of ostreedev/ostree@558720e

If we hardlink zero sized files, then any modification to them
will result in possibly *many* hardlinks needing to be reproduced
when serializing to tar, which is definitely suboptimal.

But further, it provokes a bug in our import path; when
processing derived layers, we need to handle *both* cases
of having the file in `/sysroot` be the source as well
as the destination of the hardlink in the tar stream.
  • Loading branch information
cgwalters committed Nov 9, 2022
1 parent d72f669 commit 883a79e
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 10 deletions.
8 changes: 6 additions & 2 deletions lib/src/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ static OWNERS: Lazy<Vec<(Regex, &str)>> = Lazy::new(|| {
("usr/lib/modules/.*/initramfs", "initramfs"),
("usr/lib/modules", "kernel"),
("usr/bin/(ba)?sh", "bash"),
("usr/lib.*/emptyfile.*", "bash"),
("usr/bin/hardlink.*", "testlink"),
("usr/etc/someconfig.conf", "someconfig"),
("usr/etc/polkit.conf", "a-polkit-config"),
Expand All @@ -146,6 +147,9 @@ m 0 0 755
r usr/bin/bash the-bash-shell
l usr/bin/sh bash
m 0 0 644
# Some empty files
r usr/lib/emptyfile
r usr/lib64/emptyfile2
# Should be the same object
r usr/bin/hardlink-a testlink
r usr/bin/hardlink-b testlink
Expand All @@ -163,8 +167,8 @@ m 0 0 1755
d tmp
"## };
pub const CONTENTS_CHECKSUM_V0: &str =
"3af747e156c34d08a3a2fb85b94de6999205a1d1c1c7b1993d6ce534a8918cd9";
pub static CONTENTS_V0_LEN: Lazy<usize> = Lazy::new(|| OWNERS.len());
"5e41de82f9f861fa51e53ce6dd640a260e4fb29b7657f5a3f14157e93d2c0659";
pub static CONTENTS_V0_LEN: Lazy<usize> = Lazy::new(|| OWNERS.len().checked_sub(1).unwrap());

#[derive(Debug, PartialEq, Eq)]
enum SeLabel {
Expand Down
26 changes: 19 additions & 7 deletions lib/src/tar/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,9 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
h.set_gid(meta.attribute_uint32("unix::gid") as u64);
let mode = meta.attribute_uint32("unix::mode");
h.set_mode(self.filter_mode(mode));
let mut target_header = h.clone();
target_header.set_size(0);

if instream.is_some() {
h.set_size(meta.size() as u64);
}
if !self.wrote_content.contains(checksum) {
let inserted = self.wrote_content.insert(checksum.to_string());
debug_assert!(inserted);
Expand Down Expand Up @@ -464,7 +464,7 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
}
}

Ok((path, target_header))
Ok((path, h))
}

/// Write a directory using the provided metadata.
Expand All @@ -488,9 +488,21 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> {
mut h: tar::Header,
dest: &Utf8Path,
) -> Result<()> {
h.set_entry_type(tar::EntryType::Link);
h.set_link_name(srcpath)?;
self.out.append_data(&mut h, dest, &mut std::io::empty())?;
// Query the original size first
let size = h.size().context("Querying size for hardlink append")?;
// Don't create hardlinks to zero-sized files, it's much more likely
// to result in generated tar streams from container builds resulting
// in a modified linked-to file in /sysroot, which we don't currently handle.
// And in the case where the input is *not* zero sized, we still output
// a hardlink of size zero, as this is what is normal.
h.set_size(0);
if h.entry_type() == tar::EntryType::Regular && size == 0 {
self.out.append_data(&mut h, dest, &mut std::io::empty())?;
} else {
h.set_entry_type(tar::EntryType::Link);
h.set_link_name(srcpath)?;
self.out.append_data(&mut h, dest, &mut std::io::empty())?;
}
Ok(())
}

Expand Down
4 changes: 3 additions & 1 deletion lib/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,12 @@ fn common_tar_structure() -> impl Iterator<Item = TarExpected> {

// Find various expected files
fn common_tar_contents_all() -> impl Iterator<Item = TarExpected> {
use tar::EntryType::{Directory, Link};
use tar::EntryType::{Directory, Link, Regular};
[
("boot", Directory, 0o755),
("usr", Directory, 0o755),
("usr/lib/emptyfile", Regular, 0o644),
("usr/lib64/emptyfile2", Regular, 0o644),
("usr/bin/bash", Link, 0o755),
("usr/bin/hardlink-a", Link, 0o644),
("usr/bin/hardlink-b", Link, 0o644),
Expand Down

0 comments on commit 883a79e

Please sign in to comment.