Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

commit: Try reflinks for local commits by default #3106

Merged
merged 1 commit into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 39 additions & 35 deletions src/libostree/ostree-repo-commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -586,11 +586,11 @@ _ostree_repo_bare_content_cleanup (OstreeRepoBareContent *regwrite)
}

/* Allocate an O_TMPFILE, write everything from @input to it, but
* not exceeding @length. Used for every object in archive repos,
* and content objects in all bare-type repos.
* not exceeding @length.
*/
static gboolean
create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length, GInputStream *input,
create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length,
GInputStream *original_input, GInputStream *input,
GLnxTmpfile *out_tmpf, GCancellable *cancellable,
GError **error)
{
Expand All @@ -601,40 +601,44 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length,
error))
return FALSE;

if (!glnx_try_fallocate (tmpf.fd, 0, length, error))
return FALSE;

if (G_IS_FILE_DESCRIPTOR_BASED (input))
// Try to do a reflink if possible; if we hit this case we're operating on trusted local input.
gboolean did_clone = FALSE;
if (G_IS_FILE_DESCRIPTOR_BASED (original_input))
{
int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased *)input);
if (glnx_regfile_copy_bytes (infd, tmpf.fd, (off_t)length) < 0)
return glnx_throw_errno_prefix (error, "regfile copy");
int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased *)original_input);
if (ioctl (tmpf.fd, FICLONE, infd) == 0)
Comment on lines -610 to +609
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While glnx_regfile_copy_bytes will try reflinks, that will never work because we've wrapped the input with a checksum stream. And actually since we have to checksum, we can't do copy_file_range either, etc.

Ultimately this issue will be moot with a switch to composefs where we'll use fsverity in kernel to do checksums.

{
did_clone = TRUE;
}
}
else
{
/* We used to do a g_output_stream_splice(), but there are two issues with that:
* - We want to honor the size provided, to avoid malicious content that says it's
* e.g. 10 bytes but is actually gigabytes.
* - Due to GLib bugs that pointlessly calls `poll()` on the output fd for every write
*/
gsize buf_size = MIN (length, 1048576);
g_autofree gchar *buf = g_malloc (buf_size);
guint64 remaining = length;
while (remaining > 0)
{
const gssize bytes_read
= g_input_stream_read (input, buf, MIN (remaining, buf_size), cancellable, error);
if (bytes_read < 0)
return FALSE;
else if (bytes_read == 0)
return glnx_throw (error,
"Unexpected EOF with %" G_GUINT64_FORMAT "/%" G_GUINT64_FORMAT
" bytes remaining",
remaining, length);
if (glnx_loop_write (tmpf.fd, buf, bytes_read) < 0)
return glnx_throw_errno_prefix (error, "write");
remaining -= bytes_read;
}
if (!glnx_try_fallocate (tmpf.fd, 0, length, error))
return FALSE;
}

/* We used to do a g_output_stream_splice(), but there are two issues with that:
* - We want to honor the size provided, to avoid malicious content that says it's
* e.g. 10 bytes but is actually gigabytes.
* - Due to GLib bugs that pointlessly calls `poll()` on the output fd for every write
*/
gsize buf_size = MIN (length, 1048576);
g_autofree gchar *buf = g_malloc (buf_size);
guint64 remaining = length;
while (remaining > 0)
{
const gssize bytes_read
= g_input_stream_read (input, buf, MIN (remaining, buf_size), cancellable, error);
if (bytes_read < 0)
return FALSE;
else if (bytes_read == 0)
return glnx_throw (error,
"Unexpected EOF with %" G_GUINT64_FORMAT "/%" G_GUINT64_FORMAT
" bytes remaining",
remaining, length);
if (!did_clone && glnx_loop_write (tmpf.fd, buf, bytes_read) < 0)
return glnx_throw_errno_prefix (error, "write");
remaining -= bytes_read;
}

if (!glnx_fchmod (tmpf.fd, 0644, error))
Expand Down Expand Up @@ -989,8 +993,8 @@ write_content_object (OstreeRepo *self, const char *expected_checksum, GInputStr
}
else if (repo_mode != OSTREE_REPO_MODE_ARCHIVE)
{
if (!create_regular_tmpfile_linkable_with_content (self, size, file_input, &tmpf, cancellable,
error))
if (!create_regular_tmpfile_linkable_with_content (self, size, input, file_input, &tmpf,
cancellable, error))
return FALSE;
}
else
Expand Down
13 changes: 13 additions & 0 deletions tests/kolainst/destructive/itest-label-selinux.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,29 @@ assert_not_has_file "${testbin}"
# Make a test binary that we label as shell_exec_t on disk, but should be
# reset by --selinux-policy back to bin_t
echo 'test foo' > ${testbin}
# Extend it with some real data to help test reflinks
cat < /usr/bin/bash >> "${testbin}"
# And set its label
chcon --reference co/usr/bin/true ${testbin}
# Now at this point, it should not have shared extents
filefrag -v "${testbin}" > out.txt
assert_not_file_has_content out.txt shared
rm -f out.txt
oldcon=$(getfattr --only-values -m security.selinux ${testbin})
chcon --reference co/usr/bin/bash ${testbin}
newcon=$(getfattr --only-values -m security.selinux ${testbin})
assert_not_streq "${oldcon}" "${newcon}"
# Ensure the tmpdir isn't pruned
touch co
ostree commit -b testbranch --link-checkout-speedup \
--selinux-policy co --tree=dir=co
ostree ls -X testbranch /usr/bin/foo-a-generic-binary > ls.txt
assert_file_has_content ls.txt ${oldcon}
ostree fsck
# Additionally, the commit process should have reflinked our input
filefrag -v "${testbin}" > out.txt
assert_file_has_content out.txt shared
rm -f out.txt

ostree refs --delete testbranch
rm co -rf
Expand Down
Loading