From 5776d5dcc09e5aabae1b5c1b8854c0b66522accd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 24 Jun 2017 14:06:53 +0000 Subject: [PATCH] Port to GLnxTmpfile There's lots of mechanically replacing `OtTmpFile` with `GLnxTmpfile`; the biggest changes are in the commit path. Symlink commits are now very clearly separated from regular files. Symlinks are `OtCleanupUnlinkat`, and regular files are `GLnxTmpfile`. The commit codepath separates those as `_ostree_repo_commit_path_final()` and `_ostree_repo_commit_tmpf_final()`. A nice aspect of all of this is that they both *consume* the temporary on success. This avoids an extra spurious `unlink()` call. One of the biggest bits of code motion is in `commit_loose_regfile_object()`, which no longer needs to care about symlinks. For the most parth though it's just removing conditionals. Update submodule: libglnx Closes: #958 Approved by: jlebon --- libglnx | 2 +- src/libostree/ostree-fetcher-curl.c | 16 +- src/libostree/ostree-impl-system-generator.c | 14 +- src/libostree/ostree-repo-checkout.c | 24 +- src/libostree/ostree-repo-commit.c | 382 +++++++++--------- src/libostree/ostree-repo-private.h | 26 +- src/libostree/ostree-repo-pull.c | 9 +- .../ostree-repo-static-delta-compilation.c | 54 ++- .../ostree-repo-static-delta-processing.c | 4 +- src/libostree/ostree-repo.c | 9 +- src/libotutil/ot-fs-utils.c | 58 +-- src/libotutil/ot-fs-utils.h | 26 -- src/ostree/ot-remote-cookie-util.c | 12 +- 13 files changed, 278 insertions(+), 358 deletions(-) diff --git a/libglnx b/libglnx index 32231fdb..caa51ac2 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 32231fdb5273dd2a812c61549d6c0e681c0f5d59 +Subproject commit caa51ac24ffcdffcb610bc6ccc9da964d4be74ee diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c index f483a6bb..5d35e7b7 100644 --- a/src/libostree/ostree-fetcher-curl.c +++ b/src/libostree/ostree-fetcher-curl.c @@ -99,7 +99,7 @@ struct FetcherRequest { OstreeFetcherRequestFlags flags; gboolean is_membuf; GError *caught_write_error; - OtTmpfile tmpf; + GLnxTmpfile tmpf; GString *output_buf; CURL *easy; @@ -270,9 +270,9 @@ ensure_tmpfile (FetcherRequest *req, GError **error) { if (!req->tmpf.initialized) { - if (!ot_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".", - O_WRONLY | O_CLOEXEC, &req->tmpf, - error)) + if (!glnx_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".", + O_WRONLY | O_CLOEXEC, &req->tmpf, + error)) return FALSE; } return TRUE; @@ -390,9 +390,9 @@ check_multi_info (OstreeFetcher *fetcher) glnx_set_error_from_errno (error); g_task_return_error (task, g_steal_pointer (&local_error)); } - else if (!ot_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE, - fetcher->tmpdir_dfd, tmpfile_path, - error)) + else if (!glnx_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE, + fetcher->tmpdir_dfd, tmpfile_path, + error)) g_task_return_error (task, g_steal_pointer (&local_error)); else { @@ -616,7 +616,7 @@ request_unref (FetcherRequest *req) g_ptr_array_unref (req->mirrorlist); g_free (req->filename); g_clear_error (&req->caught_write_error); - ot_tmpfile_clear (&req->tmpf); + glnx_tmpfile_clear (&req->tmpf); if (req->output_buf) g_string_free (req->output_buf, TRUE); curl_easy_cleanup (req->easy); diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c index 72f52bc5..5630348c 100644 --- a/src/libostree/ostree-impl-system-generator.c +++ b/src/libostree/ostree-impl-system-generator.c @@ -172,12 +172,11 @@ _ostree_impl_system_generator (const char *ostree_cmdline, /* Generate our bind mount unit */ const char *stateroot_var_path = glnx_strjoina ("/sysroot/ostree/deploy/", stateroot, "/var"); - glnx_fd_close int tmpfd = -1; - g_autofree char *tmppath = NULL; + g_auto(GLnxTmpfile) tmpf = { 0, }; if (!glnx_open_tmpfile_linkable_at (normal_dir_dfd, ".", O_WRONLY | O_CLOEXEC, - &tmpfd, &tmppath, error)) + &tmpf, error)) return FALSE; - g_autoptr(GOutputStream) outstream = g_unix_output_stream_new (tmpfd, FALSE); + g_autoptr(GOutputStream) outstream = g_unix_output_stream_new (tmpf.fd, FALSE); gsize bytes_written; /* This code is inspired by systemd's fstab-generator.c. * @@ -204,12 +203,11 @@ _ostree_impl_system_generator (const char *ostree_cmdline, return FALSE; g_clear_object (&outstream); /* It should be readable */ - if (fchmod (tmpfd, 0644) < 0) + if (fchmod (tmpf.fd, 0644) < 0) return glnx_throw_errno_prefix (error, "fchmod"); /* Error out if somehow it already exists, that'll help us debug conflicts */ - if (!glnx_link_tmpfile_at (normal_dir_dfd, GLNX_LINK_TMPFILE_NOREPLACE, - tmpfd, tmppath, normal_dir_dfd, - "var.mount", error)) + if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE, + normal_dir_dfd, "var.mount", error)) return FALSE; /* And ensure it's required; newer systemd will auto-inject fs dependencies diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index af5c021f..ac8d5b46 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -60,9 +60,9 @@ checkout_object_for_uncompressed_cache (OstreeRepo *self, guint32 file_mode = g_file_info_get_attribute_uint32 (src_info, "unix::mode"); file_mode &= ~(S_ISUID|S_ISGID); - g_auto(OtTmpfile) tmpf = { 0, }; - if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC, - &tmpf, error)) + g_auto(GLnxTmpfile) tmpf = { 0, }; + if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC, + &tmpf, error)) return FALSE; g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE); @@ -89,9 +89,9 @@ checkout_object_for_uncompressed_cache (OstreeRepo *self, cancellable, error)) return FALSE; - if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST, - self->uncompressed_objects_dir_fd, loose_path, - error)) + if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST, + self->uncompressed_objects_dir_fd, loose_path, + error)) return FALSE; return TRUE; @@ -254,11 +254,11 @@ create_file_copy_from_input_at (OstreeRepo *repo, } else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR) { - g_auto(OtTmpfile) tmpf = { 0, }; + g_auto(GLnxTmpfile) tmpf = { 0, }; GLnxLinkTmpfileReplaceMode replace_mode; - if (!ot_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC, - &tmpf, error)) + if (!glnx_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC, + &tmpf, error)) return FALSE; if (sepolicy_enabled && options->mode != OSTREE_REPO_CHECKOUT_MODE_USER) @@ -285,9 +285,9 @@ create_file_copy_from_input_at (OstreeRepo *repo, else replace_mode = GLNX_LINK_TMPFILE_NOREPLACE; - if (!ot_link_tmpfile_at (&tmpf, replace_mode, - destination_dfd, destination_name, - error)) + if (!glnx_link_tmpfile_at (&tmpf, replace_mode, + destination_dfd, destination_name, + error)) return FALSE; } else diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 916b99d2..d64f6481 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -134,21 +134,19 @@ ot_security_smack_reset_fd (int fd) #endif } +/* Given an O_TMPFILE regular file, link it into place. */ gboolean -_ostree_repo_commit_loose_final (OstreeRepo *self, - const char *checksum, - OstreeObjectType objtype, - int temp_dfd, - int fd, - const char *temp_filename, - GCancellable *cancellable, - GError **error) +_ostree_repo_commit_tmpf_final (OstreeRepo *self, + const char *checksum, + OstreeObjectType objtype, + GLnxTmpfile *tmpf, + GCancellable *cancellable, + GError **error) { - int dest_dfd; char tmpbuf[_OSTREE_LOOSE_PATH_MAX]; - _ostree_loose_path (tmpbuf, checksum, objtype, self->mode); + int dest_dfd; if (self->in_transaction) dest_dfd = self->commit_stagedir_fd; else @@ -158,41 +156,64 @@ _ostree_repo_commit_loose_final (OstreeRepo *self, cancellable, error)) return FALSE; - if (fd != -1) + return glnx_link_tmpfile_at (tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST, + dest_dfd, tmpbuf, error); +} + +/* Given a dfd+path combination (may be regular file or symlink), + * rename it into place. + */ +gboolean +_ostree_repo_commit_path_final (OstreeRepo *self, + const char *checksum, + OstreeObjectType objtype, + OtCleanupUnlinkat *tmp_path, + GCancellable *cancellable, + GError **error) +{ + /* The final renameat() */ + char tmpbuf[_OSTREE_LOOSE_PATH_MAX]; + _ostree_loose_path (tmpbuf, checksum, objtype, self->mode); + + int dest_dfd; + if (self->in_transaction) + dest_dfd = self->commit_stagedir_fd; + else + dest_dfd = self->objects_dir_fd; + + if (!_ostree_repo_ensure_loose_objdir_at (dest_dfd, tmpbuf, + cancellable, error)) + return FALSE; + + if (renameat (tmp_path->dfd, tmp_path->path, + dest_dfd, tmpbuf) == -1) { - if (!glnx_link_tmpfile_at (temp_dfd, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST, - fd, temp_filename, dest_dfd, tmpbuf, error)) - return FALSE; + if (errno != EEXIST) + return glnx_throw_errno_prefix (error, "Storing file '%s'", tmp_path->path); + /* Otherwise, the caller's cleanup will unlink+free */ } else { - if (G_UNLIKELY (renameat (temp_dfd, temp_filename, - dest_dfd, tmpbuf) == -1)) - { - if (errno != EEXIST) - return glnx_throw_errno_prefix (error, "Storing file '%s'", temp_filename); - else - (void) unlinkat (temp_dfd, temp_filename, 0); - } + /* The tmp path was consumed */ + ot_cleanup_unlinkat_clear (tmp_path); } return TRUE; } + /* Given either a file or symlink, apply the final metadata to it depending on * the repository mode. Note that @checksum is assumed to have been validated by * the caller. */ static gboolean -commit_loose_content_object (OstreeRepo *self, +commit_loose_regfile_object (OstreeRepo *self, const char *checksum, - const char *temp_filename, - gboolean object_is_symlink, + GLnxTmpfile *tmpf, guint32 uid, guint32 gid, guint32 mode, GVariant *xattrs, - int fd, GCancellable *cancellable, GError **error) { @@ -200,115 +221,80 @@ commit_loose_content_object (OstreeRepo *self, * automatically inherit the non-root ownership. */ if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2 - && self->target_owner_uid != -1) + && self->target_owner_uid != -1) { - if (fd != -1) - { - if (fchown (fd, self->target_owner_uid, self->target_owner_gid) < 0) - return glnx_throw_errno_prefix (error, "fchown"); - } - else if (G_UNLIKELY (fchownat (self->tmp_dir_fd, temp_filename, - self->target_owner_uid, - self->target_owner_gid, - AT_SYMLINK_NOFOLLOW) == -1)) - return glnx_throw_errno_prefix (error, "fchownat"); + if (fchown (tmpf->fd, self->target_owner_uid, self->target_owner_gid) < 0) + return glnx_throw_errno_prefix (error, "fchown"); } + else if (self->mode == OSTREE_REPO_MODE_BARE) + { + if (TEMP_FAILURE_RETRY (fchown (tmpf->fd, uid, gid)) < 0) + return glnx_throw_errno_prefix (error, "fchown"); - /* Special handling for symlinks in bare repositories */ - if (object_is_symlink && self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) - { - /* We don't store the metadata in bare-user-only, so we're done. */ + if (TEMP_FAILURE_RETRY (fchmod (tmpf->fd, mode)) < 0) + return glnx_throw_errno_prefix (error, "fchmod"); + + if (xattrs) + { + ot_security_smack_reset_fd (tmpf->fd); + if (!glnx_fd_set_all_xattrs (tmpf->fd, xattrs, cancellable, error)) + return FALSE; + } } - else if (object_is_symlink && self->mode == OSTREE_REPO_MODE_BARE) + else if (self->mode == OSTREE_REPO_MODE_BARE_USER) { - /* Now that we know the checksum is valid, apply uid/gid, mode bits, - * and extended attributes. + if (!write_file_metadata_to_xattr (tmpf->fd, uid, gid, mode, xattrs, error)) + return FALSE; + + /* Note that previously this path added `| 0755` which made every + * file executable, see + * https://github.com/ostreedev/ostree/issues/907 * - * Note, this does not apply for bare-user repos, as they store symlinks - * as regular files. + * Again here, symlinks in bare-user are a hairy special case; only do a + * chmod for a *real* regular file, otherwise we'll take the default 0644. */ - if (G_UNLIKELY (fchownat (self->tmp_dir_fd, temp_filename, - uid, gid, - AT_SYMLINK_NOFOLLOW) == -1)) - return glnx_throw_errno_prefix (error, "fchownat"); - - if (xattrs != NULL) + if (S_ISREG (mode)) { - ot_security_smack_reset_dfd_name (self->tmp_dir_fd, temp_filename); - if (!glnx_dfd_name_set_all_xattrs (self->tmp_dir_fd, temp_filename, - xattrs, cancellable, error)) - return FALSE; + const mode_t content_mode = (mode & (S_IFREG | 0775)); + if (fchmod (tmpf->fd, content_mode) < 0) + return glnx_throw_errno_prefix (error, "fchmod"); } + else + g_assert (S_ISLNK (mode)); } - else + else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) { - if (self->mode == OSTREE_REPO_MODE_BARE) - { - if (TEMP_FAILURE_RETRY (fchown (fd, uid, gid)) < 0) - return glnx_throw_errno_prefix (error, "fchown"); + guint32 invalid_modebits = (mode & ~S_IFMT) & ~0775; + if (invalid_modebits > 0) + return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository", + mode, invalid_modebits); - if (TEMP_FAILURE_RETRY (fchmod (fd, mode)) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); - - if (xattrs) - { - ot_security_smack_reset_fd (fd); - if (!glnx_fd_set_all_xattrs (fd, xattrs, cancellable, error)) - return FALSE; - } - } - else if (self->mode == OSTREE_REPO_MODE_BARE_USER) - { - if (!write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error)) - return FALSE; - - if (!object_is_symlink) - { - /* Note that previously this path added `| 0755` which made every - * file executable, see - * https://github.com/ostreedev/ostree/issues/907 - */ - const mode_t content_mode = (mode & (S_IFREG | 0775)); - if (fchmod (fd, content_mode) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); - } - } - else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY - && !object_is_symlink) - { - guint32 invalid_modebits = (mode & ~S_IFMT) & ~0775; - if (invalid_modebits > 0) - return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository", - mode, invalid_modebits); - - if (fchmod (fd, mode) < 0) - return glnx_throw_errno_prefix (error, "fchmod"); - } - - if (_ostree_repo_mode_is_bare (self->mode)) - { - /* To satisfy tools such as guile which compare mtimes - * to determine whether or not source files need to be compiled, - * set the modification time to OSTREE_TIMESTAMP. - */ - const struct timespec times[2] = { { OSTREE_TIMESTAMP, UTIME_OMIT }, { OSTREE_TIMESTAMP, 0} }; - if (TEMP_FAILURE_RETRY (futimens (fd, times)) < 0) - return glnx_throw_errno_prefix (error, "futimens"); - } - - /* Ensure that in case of a power cut, these files have the data we - * want. See http://lwn.net/Articles/322823/ - */ - if (!self->in_transaction && !self->disable_fsync) - { - if (fsync (fd) == -1) - return glnx_throw_errno_prefix (error, "fsync"); - } + if (fchmod (tmpf->fd, mode) < 0) + return glnx_throw_errno_prefix (error, "fchmod"); } - if (!_ostree_repo_commit_loose_final (self, checksum, OSTREE_OBJECT_TYPE_FILE, - self->tmp_dir_fd, fd, temp_filename, - cancellable, error)) + if (_ostree_repo_mode_is_bare (self->mode)) + { + /* To satisfy tools such as guile which compare mtimes + * to determine whether or not source files need to be compiled, + * set the modification time to OSTREE_TIMESTAMP. + */ + const struct timespec times[2] = { { OSTREE_TIMESTAMP, UTIME_OMIT }, { OSTREE_TIMESTAMP, 0} }; + if (TEMP_FAILURE_RETRY (futimens (tmpf->fd, times)) < 0) + return glnx_throw_errno_prefix (error, "futimens"); + } + + /* Ensure that in case of a power cut, these files have the data we + * want. See http://lwn.net/Articles/322823/ + */ + if (!self->in_transaction && !self->disable_fsync) + { + if (fsync (tmpf->fd) == -1) + return glnx_throw_errno_prefix (error, "fsync"); + } + + if (!_ostree_repo_commit_tmpf_final (self, checksum, OSTREE_OBJECT_TYPE_FILE, + tmpf, cancellable, error)) return FALSE; return TRUE; @@ -453,7 +439,7 @@ gboolean _ostree_repo_open_content_bare (OstreeRepo *self, const char *checksum, guint64 content_len, - OtTmpfile *out_tmpf, + GLnxTmpfile *out_tmpf, gboolean *out_have_object, GCancellable *cancellable, GError **error) @@ -471,14 +457,14 @@ _ostree_repo_open_content_bare (OstreeRepo *self, return TRUE; } - return ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - out_tmpf, error); + return glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, + out_tmpf, error); } gboolean _ostree_repo_commit_trusted_content_bare (OstreeRepo *self, const char *checksum, - OtTmpfile *tmpf, + GLnxTmpfile *tmpf, guint32 uid, guint32 gid, guint32 mode, @@ -492,57 +478,45 @@ _ostree_repo_commit_trusted_content_bare (OstreeRepo *self, if (!tmpf->initialized || tmpf->fd == -1) return TRUE; - if (!commit_loose_content_object (self, checksum, - tmpf->path, - FALSE, uid, gid, mode, - xattrs, tmpf->fd, - cancellable, error)) - return glnx_prefix_error (error, "Writing object %s.%s", checksum, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); - /* The path was consumed */ - g_clear_pointer (&tmpf->path, g_free); - tmpf->initialized = FALSE; - return TRUE; + return commit_loose_regfile_object (self, checksum, + tmpf, uid, gid, mode, xattrs, + cancellable, error); } static gboolean create_regular_tmpfile_linkable_with_content (OstreeRepo *self, guint64 length, GInputStream *input, - int *out_fd, - char **out_path, + GLnxTmpfile *out_tmpf, GCancellable *cancellable, GError **error) { - glnx_fd_close int temp_fd = -1; - g_autofree char *temp_filename = NULL; - + g_auto(GLnxTmpfile) tmpf = { 0, }; if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - &temp_fd, &temp_filename, - error)) + &tmpf, error)) return FALSE; - if (!ot_fallocate (temp_fd, length, error)) + if (!ot_fallocate (tmpf.fd, length, error)) return FALSE; if (G_IS_FILE_DESCRIPTOR_BASED (input)) { int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input); - if (glnx_regfile_copy_bytes (infd, temp_fd, (off_t)length, TRUE) < 0) + if (glnx_regfile_copy_bytes (infd, tmpf.fd, (off_t)length, TRUE) < 0) return glnx_throw_errno_prefix (error, "regfile copy"); } else { - g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (temp_fd, FALSE); + g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE); if (g_output_stream_splice (temp_out, input, 0, cancellable, error) < 0) return FALSE; } - if (fchmod (temp_fd, 0644) < 0) + if (fchmod (tmpf.fd, 0644) < 0) return glnx_throw_errno_prefix (error, "fchmod"); - *out_fd = temp_fd; temp_fd = -1; - *out_path = g_steal_pointer (&temp_filename); + *out_tmpf = tmpf; tmpf.initialized = FALSE; return TRUE; } @@ -615,47 +589,46 @@ write_content_object (OstreeRepo *self, * could potentially cause the system to make a temporary setuid * binary with trailing garbage, creating a window on the local * system where a malicious setuid binary exists. - */ - /* These variables are almost equivalent to OtTmpfile, except - * temp_filename might also be a symlink. Hence the CleanupUnlinkat - * which handles that case. + * + * We use GLnxTmpfile for regular files, and OtCleanupUnlinkat for symlinks. */ g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL }; - glnx_fd_close int temp_fd = -1; + g_auto(GLnxTmpfile) tmpf = { 0, }; gssize unpacked_size = 0; gboolean indexable = FALSE; - if ((_ostree_repo_mode_is_bare (repo_mode)) && !phys_object_is_symlink) + /* Is it a symlink physically? */ + if (phys_object_is_symlink) { - if (!create_regular_tmpfile_linkable_with_content (self, size, file_input, - &temp_fd, &tmp_unlinker.path, - cancellable, error)) - return FALSE; - } - else if (_ostree_repo_mode_is_bare (repo_mode) && phys_object_is_symlink) - { - /* Note: This will not be hit for bare-user mode because its converted to a - regular file and take the branch above */ + /* This will not be hit for bare-user or archive */ + g_assert (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY); if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd, g_file_info_get_symlink_target (file_info), &tmp_unlinker.path, cancellable, error)) return FALSE; } - else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2) + else if (repo_mode != OSTREE_REPO_MODE_ARCHIVE_Z2) + { + if (!create_regular_tmpfile_linkable_with_content (self, size, file_input, + &tmpf, cancellable, error)) + return FALSE; + } + else { g_autoptr(GVariant) file_meta = NULL; g_autoptr(GConverter) zlib_compressor = NULL; g_autoptr(GOutputStream) compressed_out_stream = NULL; g_autoptr(GOutputStream) temp_out = NULL; + g_assert (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2); + if (self->generate_sizes) indexable = TRUE; if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - &temp_fd, &tmp_unlinker.path, - error)) + &tmpf, error)) return FALSE; - temp_out = g_unix_output_stream_new (temp_fd, FALSE); + temp_out = g_unix_output_stream_new (tmpf.fd, FALSE); file_meta = _ostree_zlib_file_header_new (file_info, xattrs); @@ -679,7 +652,7 @@ write_content_object (OstreeRepo *self, if (!g_output_stream_flush (temp_out, cancellable, error)) return FALSE; - if (fchmod (temp_fd, 0644) < 0) + if (fchmod (tmpf.fd, 0644) < 0) return glnx_throw_errno_prefix (error, "fchmod"); } @@ -720,23 +693,61 @@ write_content_object (OstreeRepo *self, const guint32 uid = g_file_info_get_attribute_uint32 (file_info, "unix::uid"); const guint32 gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid"); const guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode"); - if (!commit_loose_content_object (self, actual_checksum, - tmp_unlinker.path, - object_file_type == G_FILE_TYPE_SYMBOLIC_LINK, - uid, gid, mode, - xattrs, temp_fd, - cancellable, error)) - return glnx_prefix_error (error, "Writing object %s.%s", actual_checksum, - ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); - /* Clear the unlinker, it was consumed */ - ot_cleanup_unlinkat_clear (&tmp_unlinker); + /* Is it "physically" a symlink? */ + if (phys_object_is_symlink) + { + if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) + { + /* We don't store the metadata in bare-user-only, so we're done. */ + } + else if (self->mode == OSTREE_REPO_MODE_BARE) + { + /* Now that we know the checksum is valid, apply uid/gid, mode bits, + * and extended attributes. + * + * Note, this does not apply for bare-user repos, as they store symlinks + * as regular files. + */ + if (G_UNLIKELY (fchownat (self->tmp_dir_fd, tmp_unlinker.path, + uid, gid, AT_SYMLINK_NOFOLLOW) == -1)) + return glnx_throw_errno_prefix (error, "fchownat"); + + if (xattrs != NULL) + { + ot_security_smack_reset_dfd_name (self->tmp_dir_fd, tmp_unlinker.path); + if (!glnx_dfd_name_set_all_xattrs (self->tmp_dir_fd, tmp_unlinker.path, + xattrs, cancellable, error)) + return FALSE; + } + } + else + { + /* We don't do symlinks in archive or bare-user */ + g_assert_not_reached (); + } + + if (!_ostree_repo_commit_path_final (self, actual_checksum, OSTREE_OBJECT_TYPE_FILE, + &tmp_unlinker, + cancellable, error)) + return FALSE; + } + else + { + /* This path is for regular files */ + if (!commit_loose_regfile_object (self, actual_checksum, &tmpf, + uid, gid, mode, + xattrs, + cancellable, error)) + return glnx_prefix_error (error, "Writing object %s.%s", actual_checksum, + ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); + } /* Update size metadata if configured */ if (indexable && object_file_type == G_FILE_TYPE_REGULAR) { struct stat stbuf; - if (!glnx_fstat (temp_fd, &stbuf, error)) + if (!glnx_fstat (tmpf.fd, &stbuf, error)) return FALSE; repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size); @@ -832,9 +843,9 @@ write_metadata_object (OstreeRepo *self, } /* Write the metadata to a temporary file */ - g_auto(OtTmpfile) tmpf = { 0, }; - if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - &tmpf, error)) + g_auto(GLnxTmpfile) tmpf = { 0, }; + if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, + &tmpf, error)) return FALSE; if (!ot_fallocate (tmpf.fd, len, error)) return FALSE; @@ -844,12 +855,9 @@ write_metadata_object (OstreeRepo *self, return glnx_throw_errno_prefix (error, "fchmod"); /* And commit it into place */ - if (!_ostree_repo_commit_loose_final (self, actual_checksum, objtype, - self->tmp_dir_fd, tmpf.fd, tmpf.path, - cancellable, error)) + if (!_ostree_repo_commit_tmpf_final (self, actual_checksum, objtype, + &tmpf, cancellable, error)) return FALSE; - /* The temp path was consumed */ - g_clear_pointer (&tmpf.path, g_free); if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 1bd797fa..d518e52b 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -296,20 +296,26 @@ _ostree_repo_verify_commit_internal (OstreeRepo *self, GError **error); gboolean -_ostree_repo_commit_loose_final (OstreeRepo *self, - const char *checksum, - OstreeObjectType objtype, - int temp_dfd, - int fd, - const char *temp_filename, - GCancellable *cancellable, - GError **error); +_ostree_repo_commit_tmpf_final (OstreeRepo *self, + const char *checksum, + OstreeObjectType objtype, + GLnxTmpfile *tmpf, + GCancellable *cancellable, + GError **error); + +gboolean +_ostree_repo_commit_path_final (OstreeRepo *self, + const char *checksum, + OstreeObjectType objtype, + OtCleanupUnlinkat *tmp_path, + GCancellable *cancellable, + GError **error); gboolean _ostree_repo_open_content_bare (OstreeRepo *self, const char *checksum, guint64 content_len, - OtTmpfile *out_tmpf, + GLnxTmpfile *out_tmpf, gboolean *out_have_object, GCancellable *cancellable, GError **error); @@ -317,7 +323,7 @@ _ostree_repo_open_content_bare (OstreeRepo *self, gboolean _ostree_repo_commit_trusted_content_bare (OstreeRepo *self, const char *checksum, - OtTmpfile *tmpf, + GLnxTmpfile *tmpf, guint32 uid, guint32 gid, guint32 mode, diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index fc29a2ec..5479d74b 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -941,12 +941,10 @@ content_fetch_on_complete (GObject *object, if (!have_object) { - if (!_ostree_repo_commit_loose_final (pull_data->repo, checksum, OSTREE_OBJECT_TYPE_FILE, - tmp_unlinker.dfd, -1, tmp_unlinker.path, - cancellable, error)) + if (!_ostree_repo_commit_path_final (pull_data->repo, checksum, objtype, + &tmp_unlinker, + cancellable, error)) goto out; - /* The path was consumed */ - ot_cleanup_unlinkat_clear (&tmp_unlinker); } pull_data->n_fetched_content++; } @@ -954,6 +952,7 @@ content_fetch_on_complete (GObject *object, { /* Non-mirroring path */ + /* If it appears corrupted, we'll delete it below */ if (!ostree_content_file_parse_at (TRUE, _ostree_fetcher_get_dfd (fetcher), tmp_unlinker.path, FALSE, &file_in, &file_info, &xattrs, diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 7bcf5a92..6d26a1e5 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -437,29 +437,25 @@ get_unpacked_unlinked_content (OstreeRepo *repo, GCancellable *cancellable, GError **error) { - g_autofree char *tmpname = NULL; - glnx_fd_close int fd = -1; + g_auto(GLnxTmpfile) tmpf = { 0, }; g_autoptr(GBytes) ret_content = NULL; g_autoptr(GInputStream) istream = NULL; g_autoptr(GOutputStream) out = NULL; if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, "/tmp", O_RDWR | O_CLOEXEC, - &fd, &tmpname, error)) + &tmpf, error)) return FALSE; - /* We don't need the file name */ - if (tmpname) - (void) unlinkat (AT_FDCWD, tmpname, 0); if (!ostree_repo_load_file (repo, checksum, &istream, NULL, NULL, cancellable, error)) return FALSE; - out = g_unix_output_stream_new (fd, FALSE); + out = g_unix_output_stream_new (tmpf.fd, FALSE); if (g_output_stream_splice (out, istream, G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET, cancellable, error) < 0) return FALSE; - { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (fd, FALSE, error); + { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (tmpf.fd, FALSE, error); if (!mfile) return FALSE; ret_content = g_mapped_file_get_bytes (mfile); @@ -1163,6 +1159,15 @@ get_fallback_headers (OstreeRepo *self, return TRUE; } +static inline void +glnx_tmpfile_clear_p (void *ptr) +{ + GLnxTmpfile *tmpf = ptr; + if (!tmpf) + return; + glnx_tmpfile_clear (tmpf); +} + /** * ostree_repo_static_delta_generate: * @self: Repo @@ -1213,7 +1218,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, guint64 total_compressed_size = 0; guint64 total_uncompressed_size = 0; g_autoptr(GVariantBuilder) part_headers = NULL; - g_autoptr(GArray) part_temp_fds = NULL; g_autoptr(GPtrArray) part_temp_paths = NULL; g_autoptr(GVariant) delta_descriptor = NULL; g_autoptr(GVariant) to_commit = NULL; @@ -1325,8 +1329,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self, } part_headers = g_variant_builder_new (G_VARIANT_TYPE ("a" OSTREE_STATIC_DELTA_META_ENTRY_FORMAT)); - part_temp_paths = g_ptr_array_new_with_free_func (g_free); - part_temp_fds = g_array_new (FALSE, TRUE, sizeof(int)); + part_temp_paths = g_ptr_array_new_with_free_func (glnx_tmpfile_clear_p); for (i = 0; i < builder.parts->len; i++) { OstreeStaticDeltaPartBuilder *part_builder = builder.parts->pdata[i]; @@ -1399,17 +1402,15 @@ ostree_repo_static_delta_generate (OstreeRepo *self, } else { - char *part_tempfile; - int part_temp_fd; + GLnxTmpfile *part_tmpf = g_new0 (GLnxTmpfile, 1); if (!glnx_open_tmpfile_linkable_at (tmp_dfd, ".", O_WRONLY | O_CLOEXEC, - &part_temp_fd, &part_tempfile, error)) + part_tmpf, error)) goto out; - /* Transfer tempfile ownership to arrays */ - g_array_append_val (part_temp_fds, part_temp_fd); - g_ptr_array_add (part_temp_paths, g_steal_pointer (&part_tempfile)); - part_temp_outstream = g_unix_output_stream_new (part_temp_fd, FALSE); + /* Transfer tempfile ownership */ + part_temp_outstream = g_unix_output_stream_new (part_tmpf->fd, FALSE); + g_ptr_array_add (part_temp_paths, g_steal_pointer (&part_tmpf)); } part_in = ot_variant_read (delta_part); @@ -1468,17 +1469,16 @@ ostree_repo_static_delta_generate (OstreeRepo *self, { g_autofree char *partstr = g_strdup_printf ("%u", i); /* Take ownership of the path/fd here */ - g_autofree char *path = g_steal_pointer (&part_temp_paths->pdata[i]); - glnx_fd_close int fd = g_array_index (part_temp_fds, int, i); - g_array_index (part_temp_fds, int, i) = -1; + g_auto(GLnxTmpfile) tmpf = *((GLnxTmpfile*)part_temp_paths->pdata[i]); + g_clear_pointer (&(part_temp_paths->pdata[i]), g_free); - if (fchmod (fd, 0644) < 0) + if (fchmod (tmpf.fd, 0644) < 0) { glnx_set_error_from_errno (error); goto out; } - if (!glnx_link_tmpfile_at (tmp_dfd, GLNX_LINK_TMPFILE_REPLACE, fd, path, + if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, descriptor_dfd, partstr, error)) goto out; } @@ -1538,14 +1538,6 @@ ostree_repo_static_delta_generate (OstreeRepo *self, ret = TRUE; out: - if (part_temp_fds) - for (i = 0; i < part_temp_fds->len; i++) - { - int fd = g_array_index (part_temp_fds, int, i); - if (fd == -1) - continue; - (void) close (fd); - } g_clear_pointer (&builder.parts, g_ptr_array_unref); g_clear_pointer (&builder.fallback_objects, g_ptr_array_unref); return ret; diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index b8bd6587..9141f659 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -56,7 +56,7 @@ typedef struct { GError **async_error; OstreeObjectType output_objtype; - OtTmpfile tmpf; + GLnxTmpfile tmpf; guint64 content_size; GOutputStream *content_out; GChecksum *content_checksum; @@ -281,7 +281,7 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, ret = TRUE; out: - ot_tmpfile_clear (&state->tmpf); + glnx_tmpfile_clear (&state->tmpf); g_clear_object (&state->content_out); g_clear_pointer (&state->content_checksum, g_checksum_free); return ret; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 6dcb70de..20f866b9 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -3862,8 +3862,7 @@ sign_data (OstreeRepo *self, GError **error) { gboolean ret = FALSE; - glnx_fd_close int tmp_fd = -1; - g_autofree char *tmp_path = NULL; + g_auto(GLnxTmpfile) tmpf = { 0, }; g_autoptr(GOutputStream) tmp_signature_output = NULL; gpgme_ctx_t context = NULL; g_autoptr(GBytes) ret_signature = NULL; @@ -3874,9 +3873,9 @@ sign_data (OstreeRepo *self, g_autoptr(GMappedFile) signature_file = NULL; if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_RDWR | O_CLOEXEC, - &tmp_fd, &tmp_path, error)) + &tmpf, error)) goto out; - tmp_signature_output = g_unix_output_stream_new (tmp_fd, FALSE); + tmp_signature_output = g_unix_output_stream_new (tmpf.fd, FALSE); context = ot_gpgme_new_ctx (homedir, error); if (!context) @@ -3930,7 +3929,7 @@ sign_data (OstreeRepo *self, if (!g_output_stream_close (tmp_signature_output, cancellable, error)) goto out; - signature_file = g_mapped_file_new_from_fd (tmp_fd, FALSE, error); + signature_file = g_mapped_file_new_from_fd (tmpf.fd, FALSE, error); if (!signature_file) goto out; ret_signature = g_mapped_file_get_bytes (signature_file); diff --git a/src/libotutil/ot-fs-utils.c b/src/libotutil/ot-fs-utils.c index 8ed88984..d2bde837 100644 --- a/src/libotutil/ot-fs-utils.c +++ b/src/libotutil/ot-fs-utils.c @@ -25,61 +25,6 @@ #include #include -/* Before https://github.com/GNOME/libglnx/commit/9929adc, the libglnx - * tmpfile API made it hard to clean up tmpfiles in failure cases. - * it's API breaking. Carry the fix here until we're ready to fully port. - */ -void -ot_tmpfile_clear (OtTmpfile *tmpf) -{ - if (!tmpf->initialized) - return; - if (tmpf->fd == -1) - return; - (void) close (tmpf->fd); - /* If ->path is set, we're likely aborting due to an error. Clean it up */ - if (tmpf->path) - { - (void) unlinkat (tmpf->src_dfd, tmpf->path, 0); - g_free (tmpf->path); - } -} - -gboolean -ot_open_tmpfile_linkable_at (int dfd, - const char *subpath, - int flags, - OtTmpfile *out_tmpf, - GError **error) -{ - if (!glnx_open_tmpfile_linkable_at (dfd, subpath, flags, &out_tmpf->fd, &out_tmpf->path, error)) - return FALSE; - out_tmpf->initialized = TRUE; - out_tmpf->src_dfd = dfd; - return TRUE; -} - -gboolean -ot_link_tmpfile_at (OtTmpfile *tmpf, - GLnxLinkTmpfileReplaceMode mode, - int target_dfd, - const char *target, - GError **error) -{ - g_return_val_if_fail (tmpf->initialized, FALSE); - glnx_fd_close int fd = glnx_steal_fd (&tmpf->fd); - if (!glnx_link_tmpfile_at (tmpf->src_dfd, mode, fd, tmpf->path, - target_dfd, target, error)) - { - if (tmpf->path) - (void) unlinkat (tmpf->src_dfd, tmpf->path, 0); - tmpf->initialized = FALSE; - return FALSE; - } - tmpf->initialized = FALSE; - return TRUE; -} - /* Convert a fd-relative path to a GFile* - use * for legacy code. */ @@ -213,9 +158,8 @@ ot_dfd_iter_init_allow_noent (int dfd, *out_exists = FALSE; return TRUE; } - if (!glnx_dirfd_iterator_init_take_fd (fd, dfd_iter, error)) + if (!glnx_dirfd_iterator_init_take_fd (&fd, dfd_iter, error)) return FALSE; - fd = -1; *out_exists = TRUE; return TRUE; } diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h index 0f62cb7f..3b40d337 100644 --- a/src/libotutil/ot-fs-utils.h +++ b/src/libotutil/ot-fs-utils.h @@ -25,31 +25,6 @@ G_BEGIN_DECLS -/* This is a copy of https://github.com/GNOME/libglnx/pull/46 until we - * can do a full port; see https://github.com/ostreedev/ostree/pull/861 */ -typedef struct { - gboolean initialized; - int src_dfd; - int fd; - char *path; -} OtTmpfile; -void ot_tmpfile_clear (OtTmpfile *tmpf); -G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtTmpfile, ot_tmpfile_clear); - -gboolean -ot_open_tmpfile_linkable_at (int dfd, - const char *subpath, - int flags, - OtTmpfile *out_tmpf, - GError **error); -gboolean -ot_link_tmpfile_at (OtTmpfile *tmpf, - GLnxLinkTmpfileReplaceMode flags, - int target_dfd, - const char *target, - GError **error); - - /* A little helper to call unlinkat() as a cleanup * function. Mostly only necessary to handle * deletion of temporary symlinks. @@ -76,7 +51,6 @@ ot_cleanup_unlinkat (OtCleanupUnlinkat *cleanup) } G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtCleanupUnlinkat, ot_cleanup_unlinkat); - GFile * ot_fdrel_to_gfile (int dfd, const char *path); gboolean ot_readlinkat_gfile_info (int dfd, diff --git a/src/ostree/ot-remote-cookie-util.c b/src/ostree/ot-remote-cookie-util.c index 9e152e18..a33b38bf 100644 --- a/src/ostree/ot-remote-cookie-util.c +++ b/src/ostree/ot-remote-cookie-util.c @@ -202,7 +202,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path, { gboolean found = FALSE; #ifdef HAVE_LIBCURL - g_auto(OtTmpfile) tmpf = { 0, }; + g_auto(GLnxTmpfile) tmpf = { 0, }; g_autofree char *dnbuf = NULL; const char *dn = NULL; g_autoptr(OtCookieParser) parser = NULL; @@ -212,8 +212,8 @@ ot_delete_cookie_at (int dfd, const char *jar_path, dnbuf = g_strdup (jar_path); dn = dirname (dnbuf); - if (!ot_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC, - &tmpf, error)) + if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC, + &tmpf, error)) return FALSE; while (ot_parse_cookies_next (parser)) @@ -232,9 +232,9 @@ ot_delete_cookie_at (int dfd, const char *jar_path, return glnx_throw_errno_prefix (error, "write"); } - if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, - AT_FDCWD, jar_path, - error)) + if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE, + AT_FDCWD, jar_path, + error)) return FALSE; #else GSList *cookies;