diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 45b5bc6e8b..d4a23d8f73 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -446,57 +446,39 @@ ot_fallocate (int fd, goffset size, GError **error) return TRUE; } +/* Combines a check for whether or not we already have the object with + * allocating a tempfile if we don't. Used by the static delta code. + */ gboolean _ostree_repo_open_content_bare (OstreeRepo *self, const char *checksum, guint64 content_len, - OstreeRepoContentBareCommit *out_state, - GOutputStream **out_stream, + OtTmpfile *out_tmpf, gboolean *out_have_object, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - g_autofree char *temp_filename = NULL; - g_autoptr(GOutputStream) ret_stream = NULL; gboolean have_obj; - if (!_ostree_repo_has_loose_object (self, checksum, OSTREE_OBJECT_TYPE_FILE, &have_obj, cancellable, error)) - goto out; - - if (!have_obj) + return FALSE; + /* Do we already have this object? */ + *out_have_object = have_obj; + if (have_obj) { - int fd; - - if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - &fd, &temp_filename, error)) - goto out; - - if (!ot_fallocate (fd, content_len, error)) - goto out; - - ret_stream = g_unix_output_stream_new (fd, TRUE); + /* Make sure the tempfile is unset */ + out_tmpf->initialized = 0; + return TRUE; } - ret = TRUE; - if (!have_obj) - { - out_state->temp_filename = temp_filename; - temp_filename = NULL; - out_state->fd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*)ret_stream); - if (out_stream) - *out_stream = g_steal_pointer (&ret_stream); - } - *out_have_object = have_obj; - out: - return ret; + return ot_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, - OstreeRepoContentBareCommit *state, + OtTmpfile *tmpf, guint32 uid, guint32 gid, guint32 mode, @@ -504,25 +486,22 @@ _ostree_repo_commit_trusted_content_bare (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; - - if (state->fd != -1) - { - if (!commit_loose_content_object (self, checksum, - state->temp_filename, - FALSE, uid, gid, mode, - xattrs, state->fd, - cancellable, error)) - { - g_prefix_error (error, "Writing object %s.%s: ", checksum, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE)); - goto out; - } - } + /* I don't think this is necessary, but a similar check was here previously, + * keeping it for extra redundancy. + */ + if (!tmpf->initialized || tmpf->fd == -1) + return TRUE; - ret = TRUE; - out: - g_free (state->temp_filename); - return ret; + 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; } static gboolean @@ -567,23 +546,6 @@ create_regular_tmpfile_linkable_with_content (OstreeRepo *self, return TRUE; } -/* A little helper to call unlinkat() as a cleanup - * function. Mostly only necessary to handle - * deletion of temporary symlinks. - */ -typedef struct { - int dfd; - const char *path; -} CleanupUnlinkat; - -static void -cleanup_unlinkat (CleanupUnlinkat *cleanup) -{ - if (cleanup->path) - (void) unlinkat (cleanup->dfd, cleanup->path, 0); -} -G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(CleanupUnlinkat, cleanup_unlinkat); - /* Write a content object. */ static gboolean write_content_object (OstreeRepo *self, @@ -614,29 +576,38 @@ write_content_object (OstreeRepo *self, cancellable, error)) return FALSE; - gboolean temp_file_is_regular = g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR; - gboolean temp_file_is_symlink = g_file_info_get_file_type (file_info) == G_FILE_TYPE_SYMBOLIC_LINK; - gboolean object_is_symlink = temp_file_is_symlink; + gboolean phys_object_is_symlink = FALSE; + const GFileType object_file_type = g_file_info_get_file_type (file_info); + switch (object_file_type) + { + case G_FILE_TYPE_REGULAR: + break; + case G_FILE_TYPE_SYMBOLIC_LINK: + if (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY) + phys_object_is_symlink = TRUE; + break; + default: + return glnx_throw (error, "Unsupported file type %u", object_file_type); + } + + guint64 size; - if (repo_mode == OSTREE_REPO_MODE_BARE_USER && object_is_symlink) + /* For bare-user, convert the symlink target to the input stream */ + if (repo_mode == OSTREE_REPO_MODE_BARE_USER && object_file_type == G_FILE_TYPE_SYMBOLIC_LINK) { const char *target_str = g_file_info_get_symlink_target (file_info); g_autoptr(GBytes) target = g_bytes_new (target_str, strlen (target_str) + 1); - /* For bare-user we can't store symlinks as symlinks, as symlinks don't - support user xattrs to store the ownership. So, instead store them - as regular files */ - temp_file_is_regular = TRUE; - temp_file_is_symlink = FALSE; - if (file_input != NULL) g_object_unref (file_input); /* Include the terminating zero so we can e.g. mmap this file */ file_input = g_memory_input_stream_new_from_bytes (target); + size = g_bytes_get_size (target); } - - if (!(temp_file_is_regular || temp_file_is_symlink)) - return glnx_throw (error, "Unsupported file type %u", g_file_info_get_file_type (file_info)); + else if (!phys_object_is_symlink) + size = g_file_info_get_size (file_info); + else + size = 0; /* For regular files, we create them with default mode, and only * later apply any xattrs and setuid bits. The rationale here @@ -649,31 +620,26 @@ write_content_object (OstreeRepo *self, * temp_filename might also be a symlink. Hence the CleanupUnlinkat * which handles that case. */ - g_auto(CleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL }; + g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL }; glnx_fd_close int temp_fd = -1; - g_autofree char *temp_filename = NULL; gssize unpacked_size = 0; gboolean indexable = FALSE; - if ((_ostree_repo_mode_is_bare (repo_mode)) && temp_file_is_regular) + if ((_ostree_repo_mode_is_bare (repo_mode)) && !phys_object_is_symlink) { - guint64 size = g_file_info_get_size (file_info); - if (!create_regular_tmpfile_linkable_with_content (self, size, file_input, - &temp_fd, &temp_filename, + &temp_fd, &tmp_unlinker.path, cancellable, error)) return FALSE; - tmp_unlinker.path = temp_filename; } - else if (_ostree_repo_mode_is_bare (repo_mode) && temp_file_is_symlink) + 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 */ if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd, g_file_info_get_symlink_target (file_info), - &temp_filename, + &tmp_unlinker.path, cancellable, error)) return FALSE; - tmp_unlinker.path = temp_filename; } else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2) { @@ -686,11 +652,9 @@ write_content_object (OstreeRepo *self, indexable = TRUE; if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC, - &temp_fd, &temp_filename, + &temp_fd, &tmp_unlinker.path, error)) return FALSE; - tmp_unlinker.path = temp_filename; - temp_file_is_regular = TRUE; temp_out = g_unix_output_stream_new (temp_fd, FALSE); file_meta = _ostree_zlib_file_header_new (file_info, xattrs); @@ -757,18 +721,18 @@ write_content_object (OstreeRepo *self, 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, - temp_filename, - object_is_symlink, + 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 path, it was consumed */ - tmp_unlinker.path = NULL; + /* Clear the unlinker, it was consumed */ + ot_cleanup_unlinkat_clear (&tmp_unlinker); /* Update size metadata if configured */ - if (indexable && temp_file_is_regular) + if (indexable && object_file_type == G_FILE_TYPE_REGULAR) { struct stat stbuf; diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 8f87b10387..2e9d92bfad 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -22,7 +22,7 @@ #include "ostree-repo.h" #include "ostree-remote-private.h" -#include "libglnx.h" +#include "otutil.h" G_BEGIN_DECLS @@ -294,17 +294,11 @@ _ostree_repo_commit_loose_final (OstreeRepo *self, GCancellable *cancellable, GError **error); -typedef struct { - int fd; - char *temp_filename; -} OstreeRepoContentBareCommit; - gboolean _ostree_repo_open_content_bare (OstreeRepo *self, const char *checksum, guint64 content_len, - OstreeRepoContentBareCommit *out_state, - GOutputStream **out_stream, + OtTmpfile *out_tmpf, gboolean *out_have_object, GCancellable *cancellable, GError **error); @@ -312,7 +306,7 @@ _ostree_repo_open_content_bare (OstreeRepo *self, gboolean _ostree_repo_commit_trusted_content_bare (OstreeRepo *self, const char *checksum, - OstreeRepoContentBareCommit *state, + OtTmpfile *tmpf, guint32 uid, guint32 gid, guint32 mode, diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 2c87fd6090..6f2cdadf2b 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -856,13 +856,13 @@ content_fetch_on_complete (GObject *object, g_autoptr(GVariant) xattrs = NULL; g_autoptr(GInputStream) file_in = NULL; g_autoptr(GInputStream) object_input = NULL; - g_autofree char *temp_path = NULL; + g_auto(OtCleanupUnlinkat) tmp_unlinker = { _ostree_fetcher_get_dfd (fetcher), NULL }; const char *checksum; g_autofree char *checksum_obj = NULL; OstreeObjectType objtype; gboolean free_fetch_data = TRUE; - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &temp_path, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmp_unlinker.path, error)) goto out; ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype); @@ -887,9 +887,11 @@ content_fetch_on_complete (GObject *object, if (!have_object) { if (!_ostree_repo_commit_loose_final (pull_data->repo, checksum, OSTREE_OBJECT_TYPE_FILE, - _ostree_fetcher_get_dfd (fetcher), -1, temp_path, + tmp_unlinker.dfd, -1, tmp_unlinker.path, cancellable, error)) goto out; + /* The path was consumed */ + ot_cleanup_unlinkat_clear (&tmp_unlinker); } pull_data->n_fetched_content++; } @@ -898,20 +900,16 @@ content_fetch_on_complete (GObject *object, /* Non-mirroring path */ if (!ostree_content_file_parse_at (TRUE, _ostree_fetcher_get_dfd (fetcher), - temp_path, FALSE, + tmp_unlinker.path, FALSE, &file_in, &file_info, &xattrs, cancellable, error)) - { - /* If it appears corrupted, delete it */ - (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); - goto out; - } + goto out; /* Also, delete it now that we've opened it, we'll hold * a reference to the fd. If we fail to validate or write, then * the temp space will be cleaned up. */ - (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); + ot_cleanup_unlinkat (&tmp_unlinker); if (!validate_bareuseronly_mode (pull_data, checksum, @@ -992,7 +990,7 @@ meta_fetch_on_complete (GObject *object, FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; g_autoptr(GVariant) metadata = NULL; - g_autofree char *temp_path = NULL; + g_auto(OtCleanupUnlinkat) tmp_unlinker = { _ostree_fetcher_get_dfd (fetcher), NULL }; const char *checksum; g_autofree char *checksum_obj = NULL; OstreeObjectType objtype; @@ -1006,7 +1004,7 @@ meta_fetch_on_complete (GObject *object, g_debug ("fetch of %s%s complete", checksum_obj, fetch_data->is_detached_meta ? " (detached)" : ""); - if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &temp_path, error)) + if (!_ostree_fetcher_request_to_tmpfile_finish (fetcher, result, &tmp_unlinker.path, error)) { if (g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { @@ -1049,22 +1047,24 @@ meta_fetch_on_complete (GObject *object, if (objtype == OSTREE_OBJECT_TYPE_TOMBSTONE_COMMIT) goto out; - fd = openat (_ostree_fetcher_get_dfd (fetcher), temp_path, O_RDONLY | O_CLOEXEC); + fd = openat (_ostree_fetcher_get_dfd (fetcher), tmp_unlinker.path, O_RDONLY | O_CLOEXEC); if (fd == -1) { glnx_set_error_from_errno (error); goto out; } + /* Now delete it, keeping the fd open as the last reference; see comment in + * corresponding content fetch path. + */ + ot_cleanup_unlinkat (&tmp_unlinker); + if (fetch_data->is_detached_meta) { if (!ot_util_variant_map_fd (fd, 0, G_VARIANT_TYPE ("a{sv}"), FALSE, &metadata, error)) goto out; - /* Now delete it, see comment in corresponding content fetch path */ - (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); - if (!ostree_repo_write_commit_detached_metadata (pull_data->repo, checksum, metadata, pull_data->cancellable, error)) goto out; @@ -1082,8 +1082,6 @@ meta_fetch_on_complete (GObject *object, FALSE, &metadata, error)) goto out; - (void) unlinkat (_ostree_fetcher_get_dfd (fetcher), temp_path, 0); - /* Write the commitpartial file now while we're still fetching data */ if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index a0f5126224..b8bd6587bb 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; - OstreeRepoContentBareCommit barecommitstate; + OtTmpfile tmpf; guint64 content_size; GOutputStream *content_out; GChecksum *content_checksum; @@ -281,6 +281,8 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, ret = TRUE; out: + ot_tmpfile_clear (&state->tmpf); + g_clear_object (&state->content_out); g_clear_pointer (&state->content_checksum, g_checksum_free); return ret; } @@ -419,8 +421,6 @@ do_content_open_generic (OstreeRepo *repo, if (!read_varuint64 (state, &xattr_offset, error)) goto out; - state->barecommitstate.fd = -1; - modev = g_variant_get_child_value (state->mode_dict, mode_offset); g_variant_get (modev, "(uuu)", &uid, &gid, &mode); state->uid = GUINT32_FROM_BE (uid); @@ -608,14 +608,14 @@ dispatch_open_splice_and_close (OstreeRepo *repo, { if (!_ostree_repo_open_content_bare (repo, state->checksum, state->content_size, - &state->barecommitstate, - &state->content_out, + &state->tmpf, &state->have_obj, cancellable, error)) goto out; if (!state->have_obj) { + state->content_out = g_unix_output_stream_new (state->tmpf.fd, FALSE); if (!handle_untrusted_content_checksum (repo, state, cancellable, error)) goto out; @@ -711,11 +711,12 @@ dispatch_open (OstreeRepo *repo, if (!_ostree_repo_open_content_bare (repo, state->checksum, state->content_size, - &state->barecommitstate, - &state->content_out, + &state->tmpf, &state->have_obj, cancellable, error)) goto out; + if (!state->have_obj) + state->content_out = g_unix_output_stream_new (state->tmpf.fd, FALSE); if (!handle_untrusted_content_checksum (repo, state, cancellable, error)) goto out; @@ -899,11 +900,12 @@ dispatch_close (OstreeRepo *repo, } } - if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->barecommitstate, + if (!_ostree_repo_commit_trusted_content_bare (repo, state->checksum, &state->tmpf, state->uid, state->gid, state->mode, state->xattrs, cancellable, error)) goto out; + g_clear_object (&state->content_out); } if (!dispatch_unset_read_source (repo, state, cancellable, error)) @@ -911,7 +913,6 @@ dispatch_close (OstreeRepo *repo, g_clear_pointer (&state->xattrs, g_variant_unref); g_clear_pointer (&state->content_checksum, g_checksum_free); - g_clear_object (&state->content_out); state->checksum_index++; state->output_target = NULL; diff --git a/src/libotutil/ot-fs-utils.h b/src/libotutil/ot-fs-utils.h index 26c5a49978..0f62cb7fe1 100644 --- a/src/libotutil/ot-fs-utils.h +++ b/src/libotutil/ot-fs-utils.h @@ -49,6 +49,34 @@ ot_link_tmpfile_at (OtTmpfile *tmpf, const char *target, GError **error); + +/* A little helper to call unlinkat() as a cleanup + * function. Mostly only necessary to handle + * deletion of temporary symlinks. + */ +typedef struct { + int dfd; + char *path; +} OtCleanupUnlinkat; + +static inline void +ot_cleanup_unlinkat_clear (OtCleanupUnlinkat *cleanup) +{ + g_clear_pointer (&cleanup->path, g_free); +} + +static inline void +ot_cleanup_unlinkat (OtCleanupUnlinkat *cleanup) +{ + if (cleanup->path) + { + (void) unlinkat (cleanup->dfd, cleanup->path, 0); + ot_cleanup_unlinkat_clear (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,