From bcb88f0484e61ac68b744f6a992a46c730e82b2c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Oct 2021 16:00:53 -0400 Subject: [PATCH 01/30] configure: post-release version bump --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index b5a3c82a..554cffb2 100644 --- a/configure.ac +++ b/configure.ac @@ -1,10 +1,10 @@ AC_PREREQ([2.63]) dnl To perform a release, follow the instructions in `docs/CONTRIBUTING.md`. m4_define([year_version], [2021]) -m4_define([release_version], [5]) +m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From 92ed1857ae49a5e4d16bea345b6b5bf6671bf990 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 5 Oct 2021 23:51:37 +0100 Subject: [PATCH 02/30] test-commit-sign.sh: Skip a unit test when running as an installed-test Signed-off-by: Simon McVittie --- tests/test-commit-sign.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test-commit-sign.sh b/tests/test-commit-sign.sh index c3f9ce63..e1759198 100755 --- a/tests/test-commit-sign.sh +++ b/tests/test-commit-sign.sh @@ -88,8 +88,12 @@ assert_file_has_content_literal show.txt 'Found 1 signature' echo "ok pull verify" # Run tests written in C -${OSTREE_UNINSTALLED}/tests/test-commit-sign-sh-ext -echo "ok extra C tests" +if [ -n "${OSTREE_UNINSTALLED:-}" ]; then + ${OSTREE_UNINSTALLED}/tests/test-commit-sign-sh-ext + echo "ok extra C tests" +else + echo "ok # SKIP test only available when running uninstalled" +fi # Clean things up and reinit rm repo -rf From a73a28634d9d68ae98061529e36023adf353aed7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 6 Oct 2021 12:42:56 -0400 Subject: [PATCH 03/30] Remove OstreeTlsCertInteraction bits from introspection We filter out everything named `-private.h` from scanning, which differs from the gtk-doc exclude. Eventually this will be solved when we switch to the new gir-based docs. Came up in https://github.com/ostreedev/ostree-rs/pull/34#discussion_r723337772 --- Makefile-libostree.am | 2 +- apidoc/Makefile.am | 2 +- src/libostree/ostree-fetcher-soup.c | 2 +- ...cert-interaction.h => ostree-tls-cert-interaction-private.h} | 0 src/libostree/ostree-tls-cert-interaction.c | 2 +- 5 files changed, 4 insertions(+), 4 deletions(-) rename src/libostree/{ostree-tls-cert-interaction.h => ostree-tls-cert-interaction-private.h} (100%) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 6b94f76f..1f698afc 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -146,7 +146,7 @@ endif if HAVE_LIBSOUP_CLIENT_CERTS libostree_1_la_SOURCES += \ src/libostree/ostree-tls-cert-interaction.c \ - src/libostree/ostree-tls-cert-interaction.h \ + src/libostree/ostree-tls-cert-interaction-private.h \ $(NULL) endif diff --git a/apidoc/Makefile.am b/apidoc/Makefile.am index f8aa5998..f1f8faed 100644 --- a/apidoc/Makefile.am +++ b/apidoc/Makefile.am @@ -87,7 +87,7 @@ IGNORE_HFILES= \ ostree-repo-pull-private.h \ ostree-repo-static-delta-private.h \ ostree-sysroot-private.h \ - ostree-tls-cert-interaction.h \ + ostree-tls-cert-interaction-private.h \ $(NULL) # Images to copy into HTML directory. diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index e1e5002e..5f707175 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -35,7 +35,7 @@ #include "ostree-fetcher.h" #include "ostree-fetcher-util.h" #ifdef HAVE_LIBSOUP_CLIENT_CERTS -#include "ostree-tls-cert-interaction.h" +#include "ostree-tls-cert-interaction-private.h" #endif #include "ostree-enumtypes.h" #include "ostree.h" diff --git a/src/libostree/ostree-tls-cert-interaction.h b/src/libostree/ostree-tls-cert-interaction-private.h similarity index 100% rename from src/libostree/ostree-tls-cert-interaction.h rename to src/libostree/ostree-tls-cert-interaction-private.h diff --git a/src/libostree/ostree-tls-cert-interaction.c b/src/libostree/ostree-tls-cert-interaction.c index bd90166a..7614244e 100644 --- a/src/libostree/ostree-tls-cert-interaction.c +++ b/src/libostree/ostree-tls-cert-interaction.c @@ -19,7 +19,7 @@ #include "config.h" -#include "ostree-tls-cert-interaction.h" +#include "ostree-tls-cert-interaction-private.h" struct _OstreeTlsCertInteraction { From f4be52ba2423858cf82fb0315bcf9b437c6d7f8f Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Thu, 7 Oct 2021 14:40:31 +0000 Subject: [PATCH 04/30] prepare-root: tweak log messages to clarify errors This rewords errors and log messages in the functions which take care of preparing sysroot in initramfs. Depending on the boot flow, it is possible to reach this logic with a sysroot mounted (unexpectedly) as read-only. In that case, let's clearly point out the problematic mountpoint. --- src/switchroot/ostree-prepare-root.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 6bc2c374..3116bef1 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -189,6 +189,10 @@ main(int argc, char *argv[]) err (EXIT_FAILURE, "usage: ostree-prepare-root SYSROOT"); root_arg = argv[1]; } +#ifdef USE_LIBSYSTEMD + sd_journal_send ("MESSAGE=preparing sysroot at %s", root_arg, + NULL); +#endif struct stat stbuf; if (stat ("/proc/cmdline", &stbuf) < 0) @@ -238,15 +242,20 @@ main(int argc, char *argv[]) */ const bool sysroot_readonly = sysroot_is_configured_ro (root_arg); const bool sysroot_currently_writable = !path_is_on_readonly_fs (root_arg); - #ifdef USE_LIBSYSTEMD - sd_journal_send ("MESSAGE=sysroot configured read-only: %d, currently writable: %d", - (int)sysroot_readonly, (int)sysroot_currently_writable, NULL); + sd_journal_send ("MESSAGE=filesystem at %s currently writable: %d", root_arg, + (int)sysroot_currently_writable, + NULL); + sd_journal_send ("MESSAGE=sysroot.readonly configuration value: %d", + (int)sysroot_readonly, + NULL); #endif + if (sysroot_readonly) { if (!sysroot_currently_writable) - errx (EXIT_FAILURE, "sysroot=readonly currently requires writable / in initramfs"); + errx (EXIT_FAILURE, "sysroot.readonly=true requires %s to be writable at this point", + root_arg); /* Now, /etc is not normally a bind mount, but if we have a readonly * sysroot, we still need a writable /etc. And to avoid race conditions * we ensure it's writable in the initramfs, before we switchroot at all. From 8a9737aa6e1819461e3df4d33fb0721d48424893 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 11 Oct 2021 06:52:25 +0000 Subject: [PATCH 05/30] repo/private: move OstreeRepoAutoTransaction to a boxed type This defines `OstreeRepoAutoTransaction` as a boxed type, in order to support auto-generating bindings for it. That first requires adding internal reference-counting to it, to allow freely copying/freeing references to a single transaction guard. --- src/libostree/ostree-repo-private.h | 12 ++++-- src/libostree/ostree-repo.c | 57 ++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index a2666dec..daec289c 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -525,6 +525,7 @@ _ostree_repo_verify_bindings (const char *collection_id, */ typedef struct { + gint atomic_refcount; OstreeRepo *repo; } OstreeRepoAutoTransaction; @@ -544,9 +545,14 @@ _ostree_repo_auto_transaction_commit (OstreeRepoAutoTransaction *txn, GCancellable *cancellable, GError **error); -void -_ostree_repo_auto_transaction_cleanup (void *p); +OstreeRepoAutoTransaction * +_ostree_repo_auto_transaction_ref (OstreeRepoAutoTransaction *txn); -G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoTransaction, _ostree_repo_auto_transaction_cleanup); +void +_ostree_repo_auto_transaction_unref (OstreeRepoAutoTransaction *txn); + +GType _ostree_repo_auto_transaction_get_type (void); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC (OstreeRepoAutoTransaction, _ostree_repo_auto_transaction_unref); G_END_DECLS diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 772eae26..74cea37f 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -711,10 +711,9 @@ ostree_repo_auto_lock_cleanup (OstreeRepoAutoLock *auto_lock) } } - /** * _ostree_repo_auto_transaction_start: - * @repo: an #OsreeRepo object + * @repo: (not nullable): an #OsreeRepo object * @cancellable: Cancellable * @error: a #GError * @@ -734,6 +733,7 @@ _ostree_repo_auto_transaction_start (OstreeRepo *repo, return NULL; OstreeRepoAutoTransaction *txn = g_malloc(sizeof(OstreeRepoAutoTransaction)); + txn->atomic_refcount = 1; txn->repo = g_object_ref (repo); return g_steal_pointer (&txn); @@ -741,7 +741,7 @@ _ostree_repo_auto_transaction_start (OstreeRepo *repo, /** * _ostree_repo_auto_transaction_abort: - * @txn: an #OsreeRepoAutoTransaction guard + * @txn: (not nullable): an #OsreeRepoAutoTransaction guard * @cancellable: Cancellable * @error: a #GError * @@ -770,7 +770,8 @@ _ostree_repo_auto_transaction_abort (OstreeRepoAutoTransaction *txn, /** * _ostree_repo_auto_transaction_commit: - * @txn: an #OsreeRepoAutoTransaction guard + * @txn: (not nullable): an #OsreeRepoAutoTransaction guard + * @out_stats: (out) (allow-none): transaction result statistics * @cancellable: Cancellable * @error: a #GError * @@ -799,30 +800,58 @@ _ostree_repo_auto_transaction_commit (OstreeRepoAutoTransaction *txn, } /** - * _ostree_repo_auto_transaction_cleanup: - * @p: pointer to an #OsreeRepoAutoTransaction guard + * _ostree_repo_auto_transaction_ref: + * @txn: (not nullable): an #OsreeRepoAutoTransaction guard * - * Destroy a transaction guard. If the transaction has not yet been completed, - * it gets aborted. + * Return a new reference to the transaction guard. + * + * Returns: (transfer full) (not nullable): new transaction guard reference. + */ +OstreeRepoAutoTransaction * +_ostree_repo_auto_transaction_ref (OstreeRepoAutoTransaction *txn) +{ + g_assert (txn != NULL); + + gint refcount = g_atomic_int_add (&txn->atomic_refcount, 1); + g_assert (refcount > 1); + + return txn; +} + +/** + * _ostree_repo_auto_transaction_unref: + * @txn: (transfer full): an #OsreeRepoAutoTransaction guard + * + * Unreference a transaction guard. When the last reference is gone, + * if the transaction has not yet been completed, it gets aborted. */ void -_ostree_repo_auto_transaction_cleanup (void *p) +_ostree_repo_auto_transaction_unref (OstreeRepoAutoTransaction *txn) { - if (p == NULL) + if (txn == NULL) + return; + + if (!g_atomic_int_dec_and_test (&txn->atomic_refcount)) return; - OstreeRepoAutoTransaction *txn = p; // Auto-abort only if transaction has not already been aborted/committed. if (txn->repo != NULL) { g_autoptr(GError) error = NULL; - if (!_ostree_repo_auto_transaction_abort (txn, NULL, &error)) { + if (!ostree_repo_abort_transaction (txn->repo, NULL, &error)) g_warning("Failed to auto-cleanup OSTree transaction: %s", error->message); - g_clear_object (&txn->repo); - } + + g_clear_object (&txn->repo); } + + g_free (txn); + return; } +G_DEFINE_BOXED_TYPE(OstreeRepoAutoTransaction, _ostree_repo_auto_transaction, + _ostree_repo_auto_transaction_ref, + _ostree_repo_auto_transaction_unref); + static GFile * get_remotes_d_dir (OstreeRepo *self, GFile *sysroot); From 5af2a529be0197b598545210e1a1d81a3a9d25a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9e=20Ravier?= Date: Mon, 11 Oct 2021 12:29:21 +0200 Subject: [PATCH 06/30] docs: Do not convert -- & --- to en/em-dash '--' is frequently used for command line options and was thus incorrectly rendered as a special en-dash symbol. --- docs/_config.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/_config.yml b/docs/_config.yml index 44135c82..ed1c2a63 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -6,6 +6,10 @@ url: "https://ostreedev.github.io" # url: "http://localhost:4000" permalink: /:title/ markdown: kramdown +kramdown: + typographic_symbols: + ndash: "--" + mdash: "---" # Exclude the README and the bundler files that would normally be # ignored by default. From a8eed03a1974c285cb7ddf550c7385a4783bb97d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 07/30] remote: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-remote.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-remote.c b/src/libostree/ostree-remote.c index 2b068e15..93c1a328 100644 --- a/src/libostree/ostree-remote.c +++ b/src/libostree/ostree-remote.c @@ -65,8 +65,8 @@ ostree_remote_new_dynamic (const gchar *name, { OstreeRemote *remote; - g_return_val_if_fail (name != NULL && *name != '\0', NULL); - g_return_val_if_fail (refspec_name == NULL || *refspec_name != '\0', NULL); + g_assert (name != NULL && *name != '\0'); + g_assert (refspec_name == NULL || *refspec_name != '\0'); remote = g_slice_new0 (OstreeRemote); remote->ref_count = 1; From 9a7f9c2095a1fe834793978250be402bfb8237f6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 08/30] deployment: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-deployment.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c index 558434de..6397d786 100644 --- a/src/libostree/ostree-deployment.c +++ b/src/libostree/ostree-deployment.c @@ -374,9 +374,9 @@ ostree_deployment_new (int index, OstreeDeployment *self; /* index may be -1 */ - g_return_val_if_fail (osname != NULL, NULL); - g_return_val_if_fail (csum != NULL, NULL); - g_return_val_if_fail (deployserial >= 0, NULL); + g_assert (osname != NULL); + g_assert (csum != NULL); + g_assert (deployserial >= 0); /* We can have "disconnected" deployments that don't have a bootcsum/serial */ From 520b45afdd97cab47d55aecab1407a857a7008df Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 09/30] sysroot: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-sysroot.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index be5306b7..04432cbc 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1235,12 +1235,15 @@ ostree_sysroot_get_subbootversion (OstreeSysroot *self) * ostree_sysroot_get_booted_deployment: * @self: Sysroot * + * This function may only be called if the sysroot is loaded. + * * Returns: (transfer none) (nullable): The currently booted deployment, or %NULL if none */ OstreeDeployment * ostree_sysroot_get_booted_deployment (OstreeSysroot *self) { - g_return_val_if_fail (self->loadstate == OSTREE_SYSROOT_LOAD_STATE_LOADED, NULL); + g_assert (self); + g_assert (self->loadstate == OSTREE_SYSROOT_LOAD_STATE_LOADED); return self->booted_deployment; } @@ -1390,7 +1393,8 @@ ostree_sysroot_get_repo (OstreeSysroot *self, OstreeRepo * ostree_sysroot_repo (OstreeSysroot *self) { - g_return_val_if_fail (self->loadstate >= OSTREE_SYSROOT_LOAD_STATE_LOADED, NULL); + g_assert (self); + g_assert (self->loadstate >= OSTREE_SYSROOT_LOAD_STATE_LOADED); g_assert (self->repo); return self->repo; } From 3159e04980e567a918e532e168dea89112922460 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 10/30] fetcher/soup: Fix gcc `-fanalyzer` warning In general, we're probably going to need to change most of our `g_return_if_fail` to `g_assert`. The analyzer flags that the function can return `NULL`, but the caller isn't prepared for this. In practice, let's abort. --- src/libostree/ostree-fetcher-soup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-fetcher-soup.c b/src/libostree/ostree-fetcher-soup.c index 5f707175..7df48482 100644 --- a/src/libostree/ostree-fetcher-soup.c +++ b/src/libostree/ostree-fetcher-soup.c @@ -183,7 +183,7 @@ static OstreeFetcherPendingURI * pending_uri_ref (OstreeFetcherPendingURI *pending) { gint refcount; - g_return_val_if_fail (pending != NULL, NULL); + g_assert (pending); refcount = g_atomic_int_add (&pending->ref_count, 1); g_assert (refcount > 0); return pending; From f355482e1f82ee26ba0f202ff6c05b1b14ddc858 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 09:07:41 -0400 Subject: [PATCH 11/30] static-delta: Fix probably not actually possible NULL deref Flagged by `gcc -fanalyzer`. I didn't study this really deeply but I think it's not actually reachable. Anyways, let's catch it on general principle. --- src/ostree/ot-builtin-static-delta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index ff31b574..0ce3cefb 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -669,7 +669,7 @@ ostree_builtin_static_delta (int argc, char **argv, OstreeCommandInvocation *inv return TRUE; /* Note early return */ } - if (!command->fn) + if (!command || !command->fn) { static_delta_usage (argv, TRUE); return glnx_throw (error, "Unknown \"static-delta\" subcommand '%s'", cmdname); From 54bf42c3e5f6603031f55f5e2187adb3c4f5c5da Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 09:10:59 -0400 Subject: [PATCH 12/30] utils: Fix unreachable `NULL` deref by adding assertion Again this one is just in theory, but let's add an assertion. --- src/libotutil/ot-gio-utils.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libotutil/ot-gio-utils.c b/src/libotutil/ot-gio-utils.c index f09ee8af..9ee6f7d5 100644 --- a/src/libotutil/ot-gio-utils.c +++ b/src/libotutil/ot-gio-utils.c @@ -82,10 +82,13 @@ ot_gfile_ensure_unlinked (GFile *path, GCancellable *cancellable, GError **error) { - if (unlink (gs_file_get_path_cached (path)) != 0) + g_assert (path); + const char *pathc = gs_file_get_path_cached (path); + g_assert (pathc); + if (unlink (pathc) != 0) { if (errno != ENOENT) - return glnx_throw_errno_prefix (error, "unlink(%s)", gs_file_get_path_cached (path)); + return glnx_throw_errno_prefix (error, "unlink(%s)", pathc); } return TRUE; } From 029a9d56c3bc2aaf7eb5fbaf763ae41c781c490b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 8 Oct 2021 08:59:52 -0400 Subject: [PATCH 13/30] variantutil: Fix gcc `-fanalyzer` warnin Add some not-NULL assertions for return values from glib, and upgrade some `g_return_if_fail` to `g_assert`. --- src/libotutil/ot-variant-builder.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c index 6636068e..e4347f39 100644 --- a/src/libotutil/ot-variant-builder.c +++ b/src/libotutil/ot-variant-builder.c @@ -909,6 +909,7 @@ ot_variant_builder_pre_add (OtVariantBuilderInfo *info, const GVariantMemberInfo *member_info; member_info = g_variant_type_info_member_info (info->type_info, info->n_children); + g_assert (member_info); alignment = member_info->type_info->alignment; } else if (g_variant_type_is_array (info->type)) @@ -959,6 +960,7 @@ ot_variant_builder_post_add (OtVariantBuilderInfo *info, const GVariantMemberInfo *member_info; member_info = g_variant_type_info_member_info (info->type_info, info->n_children); + g_assert (member_info); if (member_info->ending_type == G_VARIANT_MEMBER_ENDING_OFFSET) ot_variant_builder_add_child_end (info); } @@ -1085,16 +1087,13 @@ ot_variant_builder_open (OtVariantBuilder *builder, OtVariantBuilderInfo *info = builder->head; OtVariantBuilderInfo *new_info; - g_return_val_if_fail (info->n_children < info->max_items, - FALSE); - g_return_val_if_fail (!info->expected_type || + g_assert (info->n_children < info->max_items); + g_assert (!info->expected_type || g_variant_type_is_subtype_of (type, - info->expected_type), - FALSE); - g_return_val_if_fail (!info->prev_item_type || + info->expected_type)); + g_assert (!info->prev_item_type || g_variant_type_is_subtype_of (info->prev_item_type, - type), - FALSE); + type)); if (!ot_variant_builder_pre_add (info, type, error)) return FALSE; From 7ba8dbf0ccfc8435414b897e6b10d0d89e0d3494 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Oct 2021 15:49:23 -0400 Subject: [PATCH 14/30] Attempt to update packit flow to build in COPR No idea if this will really work, but at least `packit srpm` does work now. --- .packit.yaml | 18 ++++++++++++++---- ci/make-git-snapshot.sh | 26 +++++++++----------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/.packit.yaml b/.packit.yaml index 857d6ec4..68a1c724 100644 --- a/.packit.yaml +++ b/.packit.yaml @@ -1,14 +1,24 @@ # build into f34-coreos-continuous on every commit to main jobs: - - job: production_build + - job: copr_build trigger: commit metadata: - branch: main - targets: f34-coreos-continuous + branch: main + owner: "@CoreOS" + project: continuous + targets: + - centos-stream-8-aarch64 + - centos-stream-8-x86_64 + - fedora-all-aarch64 + - fedora-all-s390x + - fedora-all specfile_path: ostree.spec actions: + create-archive: + - "./ci/make-git-snapshot.sh" + - "bash -c 'ls ostree-*.tar'" # https://packit.dev/faq/#how-can-i-download-rpm-spec-file-if-it-is-not-part-of-upstream-repository post-upstream-clone: - - "wget https://src.fedoraproject.org/rpms/ostree/raw/rawhide/f/ostree.spec" + - "curl -LO https://src.fedoraproject.org/rpms/ostree/raw/rawhide/f/ostree.spec" # we don't want any downstream patches - "sed -ie 's/^Patch/# Patch/g' ostree.spec" diff --git a/ci/make-git-snapshot.sh b/ci/make-git-snapshot.sh index 1a137bff..67cf14c9 100755 --- a/ci/make-git-snapshot.sh +++ b/ci/make-git-snapshot.sh @@ -1,31 +1,23 @@ -#!/bin/sh +#!/bin/bash +set -xeuo pipefail -srcdir=$1 -shift -PKG_VER=$1 -shift -GITREV=$1 -shift +TOP=$(git rev-parse --show-toplevel) +GITREV=$(git rev-parse HEAD) +gitdescribe=$(git describe --always --tags $GITREV) +version=$(echo "$gitdescribe" | sed -e 's,-,\.,g' -e 's,^v,,') +name=$(basename $(pwd)) +PKG_VER="${name}-${version}" TARFILE=${PKG_VER}.tar TARFILE_TMP=${TARFILE}.tmp -set -x -set -e - -test -n "${srcdir}" -test -n "${PKG_VER}" -test -n "${GITREV}" - -TOP=$(git rev-parse --show-toplevel) - echo "Archiving ${PKG_VER} at ${GITREV} to ${TARFILE_TMP}" (cd ${TOP}; git archive --format=tar --prefix=${PKG_VER}/ ${GITREV}) > ${TARFILE_TMP} ls -al ${TARFILE_TMP} (cd ${TOP}; git submodule status) | while read line; do rev=$(echo ${line} | cut -f 1 -d ' '); path=$(echo ${line} | cut -f 2 -d ' ') echo "Archiving ${path} at ${rev}" - (cd ${srcdir}/${path}; git archive --format=tar --prefix=${PKG_VER}/${path}/ ${rev}) > submodule.tar + (cd ${TOP}/${path}; git archive --format=tar --prefix=${PKG_VER}/${path}/ ${rev}) > submodule.tar tar -A -f ${TARFILE_TMP} submodule.tar rm submodule.tar done From baa57ffe0d383256429d9091532a250e239c760c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Oct 2021 12:40:14 -0400 Subject: [PATCH 15/30] libglnx: Bump to ef502aabf7d3a0d37f9c4d228f870ac93404447b Various fixes there, including one for `gcc -fanalyzer`. Update submodule: libglnx --- libglnx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libglnx b/libglnx index 1dd01d5e..ef502aab 160000 --- a/libglnx +++ b/libglnx @@ -1 +1 @@ -Subproject commit 1dd01d5ef172fbe7cb385c91ee2a3740962e8074 +Subproject commit ef502aabf7d3a0d37f9c4d228f870ac93404447b From fda41e8d249e479a02f67c7b3e31c98f0bb8d553 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 14 Oct 2021 10:40:39 -0400 Subject: [PATCH 16/30] ci: Enable -fanalyzer Followup to https://github.com/ostreedev/ostree/pull/2463 One thing I noticed here is we lost usage of `build-check.sh` which also invokes `clang`, which doesn't speak `-fanalyzer` and would be broken by this if we try to enable `build-check.sh` again. But that can come later. --- ci/build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build.sh b/ci/build.sh index 2afcd018..ffdeba01 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -30,6 +30,6 @@ esac # always fail on warnings; https://github.com/ostreedev/ostree/pull/971 # NB: this disables the default set of flags from configure.ac -export CFLAGS="-Wall -Werror ${CFLAGS:-}" +export CFLAGS="-Wall -Werror -fanalyzer ${CFLAGS:-}" build --enable-gtk-doc ${CONFIGOPTS:-} From 58dc6a08b49384cbde3346f6b0675d684fe7c8db Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 18 Oct 2021 11:44:44 -0400 Subject: [PATCH 17/30] tests/rollsum: Use `g_malloc` not `malloc` To pacify gcc's `-fanalyzer`. --- tests/test-rollsum.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test-rollsum.c b/tests/test-rollsum.c index 08c78b66..08aea235 100644 --- a/tests/test-rollsum.c +++ b/tests/test-rollsum.c @@ -75,8 +75,8 @@ test_rollsum (void) #define MAX_BUFFER_SIZE 1000000 gsize i; int len; - g_autofree unsigned char *a = malloc (MAX_BUFFER_SIZE); - g_autofree unsigned char *b = malloc (MAX_BUFFER_SIZE); + g_autofree unsigned char *a = g_malloc (MAX_BUFFER_SIZE); + g_autofree unsigned char *b = g_malloc (MAX_BUFFER_SIZE); g_autoptr(GRand) rand = g_rand_new (); /* These two buffers produce the same crc32. */ From f93d96620c63a5999b87bdd78456d21970a15c4f Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 19 Oct 2021 08:03:09 +0000 Subject: [PATCH 18/30] tests/var-mount: tweak test setup This reworks the var-mount destructive test in order to properly use the datadir for the current stateroot instead of a duplicated one. In turn, it ensures that the resulting `var.mount` after reboot is correctly pointing to the same location which hosted `/var` on the previous boot. --- tests/kolainst/destructive/var-mount.sh | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/kolainst/destructive/var-mount.sh b/tests/kolainst/destructive/var-mount.sh index 86c69fd1..ff615f88 100755 --- a/tests/kolainst/destructive/var-mount.sh +++ b/tests/kolainst/destructive/var-mount.sh @@ -6,12 +6,9 @@ set -xeuo pipefail case "${AUTOPKGTEST_REBOOT_MARK:-}" in "") - require_writable_sysroot - # Hack this off for now - chattr -i /sysroot - cp -a /var /sysroot/myvar - touch /sysroot/myvar/somenewfile - echo '/sysroot/myvar /var none bind 0 0' >> /etc/fstab + touch "/var/somenewfile" + stateroot=$(ostree admin status 2> /dev/null | grep '^\*' | cut -d ' ' -f2 || true) + echo "/sysroot/ostree/deploy/${stateroot}/var /var none bind 0 0" >> /etc/fstab /tmp/autopkgtest-reboot "2" ;; "2") From 848fe542afd8e475a8126f3c5478a4c9b37ce1be Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 26 Oct 2021 10:12:27 +0000 Subject: [PATCH 19/30] prepare-root: make all mount operations silent This adds a `MS_SILENT` flag to all `mount(2)` calls, reducing the amount of kernel logs produced on each boot. Those messages do not contain actionable details, and in the "mount plus read-only remount" case they can easily become highly redundant. --- src/switchroot/ostree-prepare-root.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 3116bef1..20b8685d 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -201,7 +201,7 @@ main(int argc, char *argv[]) err (EXIT_FAILURE, "stat(\"/proc/cmdline\") failed"); /* We need /proc mounted for /proc/cmdline and realpath (on musl) to * work: */ - if (mount ("proc", "/proc", "proc", 0, NULL) < 0) + if (mount ("proc", "/proc", "proc", MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to mount proc on /proc"); we_mounted_proc = 1; } @@ -224,11 +224,11 @@ main(int argc, char *argv[]) * work-around. * * https://bugzilla.redhat.com/show_bug.cgi?id=847418 */ - if (mount (NULL, "/", NULL, MS_REC|MS_PRIVATE, NULL) < 0) + if (mount (NULL, "/", NULL, MS_REC | MS_PRIVATE | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to make \"/\" private mount"); /* Make deploy_path a bind mount, so we can move it later */ - if (mount (deploy_path, deploy_path, NULL, MS_BIND, NULL) < 0) + if (mount (deploy_path, deploy_path, NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to make initial bind mount %s", deploy_path); /* chdir to our new root. We need to do this after bind-mounting it over @@ -260,7 +260,7 @@ main(int argc, char *argv[]) * sysroot, we still need a writable /etc. And to avoid race conditions * we ensure it's writable in the initramfs, before we switchroot at all. */ - if (mount ("etc", "etc", NULL, MS_BIND, NULL) < 0) + if (mount ("etc", "etc", NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to make /etc a bind mount"); /* Pass on the fact that we discovered a readonly sysroot to ostree-remount.service */ int fd = open (_OSTREE_SYSROOT_READONLY_STAMP, O_WRONLY | O_CREAT | O_CLOEXEC, 0644); @@ -281,7 +281,7 @@ main(int argc, char *argv[]) mount_var = true; /* Link to the deployment's /var */ - if (mount_var && mount ("../../var", "var", NULL, MS_BIND, NULL) < 0) + if (mount_var && mount ("../../var", "var", NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount ../../var to var"); char srcpath[PATH_MAX]; @@ -293,7 +293,7 @@ main(int argc, char *argv[]) if (lstat ("boot", &stbuf) == 0 && S_ISDIR (stbuf.st_mode)) { snprintf (srcpath, sizeof(srcpath), "%s/boot", root_mountpoint); - if (mount (srcpath, "boot", NULL, MS_BIND, NULL) < 0) + if (mount (srcpath, "boot", NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount %s to boot", srcpath); } } @@ -314,15 +314,15 @@ main(int argc, char *argv[]) err (EXIT_FAILURE, "failed to remount rootfs writable (for overlayfs)"); } - if (mount ("overlay", "usr", "overlay", 0, usr_ovl_options) < 0) + if (mount ("overlay", "usr", "overlay", MS_SILENT, usr_ovl_options) < 0) err (EXIT_FAILURE, "failed to mount /usr overlayfs"); } else { /* Otherwise, a read-only bind mount for /usr */ - if (mount ("usr", "usr", NULL, MS_BIND, NULL) < 0) + if (mount ("usr", "usr", NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount (class:readonly) /usr"); - if (mount ("usr", "usr", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY, NULL) < 0) + if (mount ("usr", "usr", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount (class:readonly) /usr"); } @@ -364,13 +364,13 @@ main(int argc, char *argv[]) if (mkdir ("/sysroot.tmp", 0755) < 0) err (EXIT_FAILURE, "couldn't create temporary sysroot /sysroot.tmp"); - if (mount (deploy_path, "/sysroot.tmp", NULL, MS_MOVE, NULL) < 0) + if (mount (deploy_path, "/sysroot.tmp", NULL, MS_MOVE | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to MS_MOVE '%s' to '/sysroot.tmp'", deploy_path); - if (mount (root_mountpoint, "sysroot", NULL, MS_MOVE, NULL) < 0) + if (mount (root_mountpoint, "sysroot", NULL, MS_MOVE | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to MS_MOVE '%s' to 'sysroot'", root_mountpoint); - if (mount (".", root_mountpoint, NULL, MS_MOVE, NULL) < 0) + if (mount (".", root_mountpoint, NULL, MS_MOVE | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to MS_MOVE %s to %s", deploy_path, root_mountpoint); if (rmdir ("/sysroot.tmp") < 0) @@ -385,7 +385,7 @@ main(int argc, char *argv[]) * at the very start (perhaps down the line systemd will have compile/runtime option * to say that the initramfs environment did everything right from the start). */ - if (mount ("none", "sysroot", NULL, MS_PRIVATE, NULL) < 0) + if (mount ("none", "sysroot", NULL, MS_PRIVATE | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "remounting 'sysroot' private"); if (running_as_pid1) From ca84da679a170fcb1b2ab59a59ec45e014530996 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 26 Oct 2021 12:12:48 +0000 Subject: [PATCH 20/30] prepare-root: check return codes for errors when assembling paths This adds checks around all `snprintf` calls in order to detect failures and gracefully abort. --- src/switchroot/ostree-prepare-root.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 20b8685d..eb1159f4 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -135,7 +135,8 @@ resolve_deploy_path (const char * root_mountpoint) if (!ostree_target) errx (EXIT_FAILURE, "No OSTree target; expected ostree=/ostree/boot.N/..."); - snprintf (destpath, sizeof(destpath), "%s/%s", root_mountpoint, ostree_target); + if (snprintf (destpath, sizeof(destpath), "%s/%s", root_mountpoint, ostree_target) < 0) + err (EXIT_FAILURE, "failed to assemble ostree target path"); if (lstat (destpath, &stbuf) < 0) err (EXIT_FAILURE, "Couldn't find specified OSTree root '%s'", destpath); if (!S_ISLNK (stbuf.st_mode)) @@ -287,12 +288,14 @@ main(int argc, char *argv[]) char srcpath[PATH_MAX]; /* If /boot is on the same partition, use a bind mount to make it visible * at /boot inside the deployment. */ - snprintf (srcpath, sizeof(srcpath), "%s/boot/loader", root_mountpoint); + if (snprintf (srcpath, sizeof(srcpath), "%s/boot/loader", root_mountpoint) < 0) + err (EXIT_FAILURE, "failed to assemble /boot/loader path"); if (lstat (srcpath, &stbuf) == 0 && S_ISLNK (stbuf.st_mode)) { if (lstat ("boot", &stbuf) == 0 && S_ISDIR (stbuf.st_mode)) { - snprintf (srcpath, sizeof(srcpath), "%s/boot", root_mountpoint); + if (snprintf (srcpath, sizeof(srcpath), "%s/boot", root_mountpoint) < 0) + err (EXIT_FAILURE, "failed to assemble /boot path"); if (mount (srcpath, "boot", NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to bind mount %s to boot", srcpath); } From 7c17daad175a9e8bd876c3c548db0bdd4449b895 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 26 Oct 2021 16:27:22 +0000 Subject: [PATCH 21/30] prepare-root: get rid of a global variable This moves a global mutable variable to a smaller local scope, as it is not really used outside of that. --- src/switchroot/ostree-prepare-root.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 20b8685d..46283f3f 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -81,9 +81,6 @@ #include "ostree-mount-util.h" -/* Initialized early in main */ -static bool running_as_pid1; - static inline bool sysroot_is_configured_ro (const char *sysroot) { @@ -175,7 +172,7 @@ main(int argc, char *argv[]) * - Quiet logging as there's no journal * etc. */ - running_as_pid1 = (getpid () == 1); + bool running_as_pid1 = (getpid () == 1); const char *root_arg = NULL; bool we_mounted_proc = false; From 63d0c4c78130154c95c6a8ff1fb231936d9fa607 Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Mon, 1 Nov 2021 09:09:58 +0000 Subject: [PATCH 22/30] prepare-root: check for read-only sysroot status early on This moves read-only sysroot checks upfront, so that they are not intermixed with mount operations. It has no immediate side-effects, but allow these check to be independent from the rest of the mounting logic (and future changes to it). --- src/switchroot/ostree-prepare-root.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index aa7d5a4e..a99d884d 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -216,6 +216,20 @@ main(int argc, char *argv[]) err (EXIT_FAILURE, "failed to umount proc from /proc"); } + /* Query the repository configuration - this is an operating system builder + * choice. More info: https://github.com/ostreedev/ostree/pull/1767 + */ + const bool sysroot_readonly = sysroot_is_configured_ro (root_arg); + const bool sysroot_currently_writable = !path_is_on_readonly_fs (root_arg); +#ifdef USE_LIBSYSTEMD + sd_journal_send ("MESSAGE=filesystem at %s currently writable: %d", root_arg, + (int)sysroot_currently_writable, + NULL); + sd_journal_send ("MESSAGE=sysroot.readonly configuration value: %d", + (int)sysroot_readonly, + NULL); +#endif + /* Work-around for a kernel bug: for some reason the kernel * refuses switching root if any file systems are mounted * MS_SHARED. Hence remount them MS_PRIVATE here as a @@ -235,20 +249,6 @@ main(int argc, char *argv[]) if (chdir (deploy_path) < 0) err (EXIT_FAILURE, "failed to chdir to deploy_path"); - /* Query the repository configuration - this is an operating system builder - * choice. More info: https://github.com/ostreedev/ostree/pull/1767 - */ - const bool sysroot_readonly = sysroot_is_configured_ro (root_arg); - const bool sysroot_currently_writable = !path_is_on_readonly_fs (root_arg); -#ifdef USE_LIBSYSTEMD - sd_journal_send ("MESSAGE=filesystem at %s currently writable: %d", root_arg, - (int)sysroot_currently_writable, - NULL); - sd_journal_send ("MESSAGE=sysroot.readonly configuration value: %d", - (int)sysroot_readonly, - NULL); -#endif - if (sysroot_readonly) { if (!sysroot_currently_writable) From c553b5c69a9112d6e74919e4eab60a55b07feece Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 25 Oct 2021 07:07:28 +0000 Subject: [PATCH 23/30] prepare-root: Set up sysroot readonly in initramfs Let's ensure things are right from the start in the initramfs; this closes off various race conditions. Followup to https://github.com/ostreedev/ostree/pull/2113/commits/35642259175973617da937f3cab6ce5f13c95077 Closes: https://github.com/ostreedev/ostree/issues/2115 --- src/switchroot/ostree-prepare-root.c | 90 ++++++++++++++++++++-------- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index a99d884d..f48fc528 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -166,6 +166,8 @@ pivot_root(const char * new_root, const char * put_old) int main(int argc, char *argv[]) { + char srcpath[PATH_MAX]; + /* If we're pid 1, that means there's no initramfs; in this situation * various defaults change: * @@ -249,17 +251,13 @@ main(int argc, char *argv[]) if (chdir (deploy_path) < 0) err (EXIT_FAILURE, "failed to chdir to deploy_path"); + /* This will result in a system with /sysroot read-only. Thus, two additional + * writable bind-mounts (for /etc and /var) are required later on. */ if (sysroot_readonly) { if (!sysroot_currently_writable) errx (EXIT_FAILURE, "sysroot.readonly=true requires %s to be writable at this point", root_arg); - /* Now, /etc is not normally a bind mount, but if we have a readonly - * sysroot, we still need a writable /etc. And to avoid race conditions - * we ensure it's writable in the initramfs, before we switchroot at all. - */ - if (mount ("etc", "etc", NULL, MS_BIND | MS_SILENT, NULL) < 0) - err (EXIT_FAILURE, "failed to make /etc a bind mount"); /* Pass on the fact that we discovered a readonly sysroot to ostree-remount.service */ int fd = open (_OSTREE_SYSROOT_READONLY_STAMP, O_WRONLY | O_CREAT | O_CLOEXEC, 0644); if (fd < 0) @@ -267,23 +265,8 @@ main(int argc, char *argv[]) (void) close (fd); } - /* Default to true, but in the systemd case, default to false because it's handled by - * ostree-system-generator. */ - bool mount_var = true; -#ifdef HAVE_SYSTEMD_AND_LIBMOUNT - mount_var = false; -#endif - - /* file in /run can override the default behaviour so that we definitely mount /var */ - if (lstat (INITRAMFS_MOUNT_VAR, &stbuf) == 0) - mount_var = true; - - /* Link to the deployment's /var */ - if (mount_var && mount ("../../var", "var", NULL, MS_BIND | MS_SILENT, NULL) < 0) - err (EXIT_FAILURE, "failed to bind mount ../../var to var"); - - char srcpath[PATH_MAX]; - /* If /boot is on the same partition, use a bind mount to make it visible + /* Prepare /boot. + * If /boot is on the same partition, use a bind mount to make it visible * at /boot inside the deployment. */ if (snprintf (srcpath, sizeof(srcpath), "%s/boot/loader", root_mountpoint) < 0) err (EXIT_FAILURE, "failed to assemble /boot/loader path"); @@ -298,9 +281,23 @@ main(int argc, char *argv[]) } } - /* Do we have a persistent overlayfs for /usr? If so, mount it now. */ + /* Prepare /etc. + * No action required if sysroot is writable. Otherwise, a bind-mount for + * the deployment needs to be created and remounted as read/write. */ + if (sysroot_readonly) + { + /* Bind-mount /etc (at deploy path), and remount as writable. */ + if (mount ("etc", "etc", NULL, MS_BIND | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to prepare /etc bind-mount at %s", srcpath); + if (mount ("etc", "etc", NULL, MS_BIND | MS_REMOUNT | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to make writable /etc bind-mount at %s", srcpath); + } + + /* Prepare /usr. + * It may be either just a read-only bind-mount, or a persistent overlayfs. */ if (lstat (".usr-ovl-work", &stbuf) == 0) { + /* Do we have a persistent overlayfs for /usr? If so, mount it now. */ const char usr_ovl_options[] = "lowerdir=usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work"; /* Except overlayfs barfs if we try to mount it on a read-only @@ -326,6 +323,38 @@ main(int argc, char *argv[]) err (EXIT_FAILURE, "failed to bind mount (class:readonly) /usr"); } + /* Prepare /var. + * When a read-only sysroot is configured, this adds a dedicated bind-mount (to itself) + * so that the stateroot location stays writable. */ + if (sysroot_readonly) + { + /* Bind-mount /var (at stateroot path), and remount as writable. */ + if (mount ("../../var", "../../var", NULL, MS_BIND | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to prepare /var bind-mount at %s", srcpath); + if (mount ("../../var", "../../var", NULL, MS_BIND | MS_REMOUNT | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to make writable /var bind-mount at %s", srcpath); + } + + /* When running under systemd, /var will be handled by a 'var.mount' unit outside + * of initramfs. + * Systemd auto-detection can be overridden by a marker file under /run. */ +#ifdef HAVE_SYSTEMD_AND_LIBMOUNT + bool mount_var = false; +#else + bool mount_var = true; +#endif + if (lstat (INITRAMFS_MOUNT_VAR, &stbuf) == 0) + mount_var = true; + + /* If required, bind-mount `/var` in the deployment to the "stateroot", which is + * the shared persistent directory for a set of deployments. More info: + * https://ostreedev.github.io/ostree/deployment/#stateroot-aka-osname-group-of-deployments-that-share-var + */ + if (mount_var) + { + if (mount ("../../var", "var", NULL, MS_BIND | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to bind mount ../../var to var"); + } /* We only stamp /run now if we're running in an initramfs, i.e. we're * not pid 1. Otherwise it's handled later via ostree-system-generator. @@ -375,6 +404,19 @@ main(int argc, char *argv[]) if (rmdir ("/sysroot.tmp") < 0) err (EXIT_FAILURE, "couldn't remove temporary sysroot /sysroot.tmp"); + + if (sysroot_readonly) + { + if (mount ("sysroot", "sysroot", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL) < 0) + err (EXIT_FAILURE, "failed to make /sysroot read-only"); + + /* TODO(lucab): This will make the final '/' read-only. + * Stabilize read-only '/sysroot' first, then enable this additional hardening too. + * + * if (mount (".", ".", NULL, MS_BIND | MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL) < 0) + * err (EXIT_FAILURE, "failed to make / read-only"); + */ + } } /* The /sysroot mount needs to be private to avoid having a mount for e.g. /var/cache From 48f582b918fc87af32d3d0c0cecf3ca5fe90a90b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0imon=20=28Simon=29=20Rataj?= Date: Wed, 3 Nov 2021 21:02:53 +0100 Subject: [PATCH 24/30] Added Fedora Kinoite link --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 234ad6c6..8fa17f94 100644 --- a/README.md +++ b/README.md @@ -51,10 +51,11 @@ projects. For Debian/apt, see also https://github.com/stb-tester/apt2ostree and the LWN article [Merkle trees and build systems](https://lwn.net/Articles/821367/). -Fedora derivatives use rpm-ostree (noted below); there are 3 variants using OSTree: +Fedora derivatives use rpm-ostree (noted below); there are 4 variants using OSTree: - [Fedora CoreOS](https://getfedora.org/en/coreos/) - [Fedora Silverblue](https://silverblue.fedoraproject.org/) + - [Fedora Kinoite](https://kinoite.fedoraproject.org/) - [Fedora IoT](https://iot.fedoraproject.org/) Red Hat Enterprise Linux CoreOS is a derivative of Fedora CoreOS, used in [OpenShift 4](https://try.openshift.com/). From adc097a2edb1b7aaf5604043b4b1d5bd6ef8a308 Mon Sep 17 00:00:00 2001 From: Valentin David Date: Tue, 2 Nov 2021 19:49:04 +0100 Subject: [PATCH 25/30] lib: Fix a bad call to g_file_get_child In Glib, since commit 3a6e8bc8876e149c36b6b14c6a25a718edb581ed, `g_file_get_child` does not accept absolute path as paramater anymore. The broken assertion was encountered during `ostree admin deploy` command for the checkout of subpath `etc`. Example of error log: ``` (ostree admin deploy:1640): GLib-GIO-CRITICAL **: 03:42:00.570: g_file_get_child: assertion '!g_path_is_absolute (name)' failed (ostree admin deploy:1640): GLib-GIO-CRITICAL **: 03:42:00.570: g_file_query_info: assertion 'G_IS_FILE (file)' failed ** OSTree:ERROR:src/ostree/ot-main.c:232:ostree_run: assertion failed: (success || error) Bail out! OSTree:ERROR:src/ostree/ot-main.c:232:ostree_run: assertion failed: (success || error) ``` --- src/libostree/ostree-repo-checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index eadaf905..93e4311d 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -1389,7 +1389,7 @@ ostree_repo_checkout_at (OstreeRepo *self, g_autoptr(GFile) target_dir = NULL; if (strcmp (options->subpath, "/") != 0) - target_dir = g_file_get_child (commit_root, options->subpath); + target_dir = g_file_resolve_relative_path (commit_root, options->subpath); else target_dir = g_object_ref (commit_root); g_autoptr(GFileInfo) target_info = From 3b338df956767fbae591941b9f4b7bb8f4a02caa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 9 Nov 2021 09:21:52 -0500 Subject: [PATCH 26/30] ci: Require `libcap2-bin` for `capsh` This was previously pulled in indirectly, but it looks like we need to require it explicitly in newer Ubuntu. --- ci/gh-install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/gh-install.sh b/ci/gh-install.sh index 25f451a0..9902a94e 100755 --- a/ci/gh-install.sh +++ b/ci/gh-install.sh @@ -84,6 +84,7 @@ case "$ID" in libsoup2.4-dev libsystemd-dev libtool + libcap2-bin procps python3 python3-yaml From 9c1fe55bbc15218e09fe3895aeb0ce9e185fc8d9 Mon Sep 17 00:00:00 2001 From: Ryan Gonzalez Date: Thu, 11 Nov 2021 18:07:06 -0600 Subject: [PATCH 27/30] lib: Avoid dereferencing NULL error values Otherwise, this will segfault when callers don't need any exact errors. Signed-off-by: Ryan Gonzalez --- src/libostree/ostree-repo-static-delta-core.c | 12 +++++++++--- src/libostree/ostree-sign-ed25519.c | 10 +++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index d8c33b7c..084c20cd 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -457,9 +457,15 @@ ostree_repo_static_delta_execute_offline_with_signature (OstreeRepo *self, if (sign) { - verified = _ostree_repo_static_delta_verify_signature (self, meta_fd, sign, NULL, error); - if (*error) - return FALSE; + g_autoptr(GError) local_error = NULL; + + verified = _ostree_repo_static_delta_verify_signature (self, meta_fd, sign, NULL, &local_error); + if (local_error != NULL) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } + if (!verified) return glnx_throw (error, "Delta signature verification failed"); } diff --git a/src/libostree/ostree-sign-ed25519.c b/src/libostree/ostree-sign-ed25519.c index d728afde..1eaff6a7 100644 --- a/src/libostree/ostree-sign-ed25519.c +++ b/src/libostree/ostree-sign-ed25519.c @@ -487,12 +487,16 @@ _load_pk_from_stream (OstreeSign *self, while (TRUE) { gsize len = 0; - g_autofree char *line = g_data_input_stream_read_line (key_data_in, &len, NULL, error); g_autoptr (GVariant) pk = NULL; gboolean added = FALSE; + g_autoptr(GError) local_error = NULL; + g_autofree char *line = g_data_input_stream_read_line (key_data_in, &len, NULL, &local_error); - if (*error != NULL) - return FALSE; + if (local_error != NULL) + { + g_propagate_error (error, g_steal_pointer (&local_error)); + return FALSE; + } if (line == NULL) return ret; From 47d32d9eadcbd322a4d6f4345259167a665b51ec Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Thu, 18 Nov 2021 10:59:46 -0700 Subject: [PATCH 28/30] lib/prune: Avoid unnecessary object serialization `repo_prune_internal` was deserializing each object and passing the components to `maybe_prune_loose_object`, which promptly reserialized it. --- src/libostree/ostree-repo-prune.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/libostree/ostree-repo-prune.c b/src/libostree/ostree-repo-prune.c index c4ce64ab..82fcf639 100644 --- a/src/libostree/ostree-repo-prune.c +++ b/src/libostree/ostree-repo-prune.c @@ -39,17 +39,17 @@ typedef struct { } OtPruneData; static gboolean -maybe_prune_loose_object (OtPruneData *data, - OstreeRepoPruneFlags flags, - const char *checksum, - OstreeObjectType objtype, - GCancellable *cancellable, - GError **error) +maybe_prune_loose_object (OtPruneData *data, + OstreeRepoPruneFlags flags, + GVariant *key, + GCancellable *cancellable, + GError **error) { gboolean reachable = FALSE; - g_autoptr(GVariant) key = NULL; + const char *checksum; + OstreeObjectType objtype; - key = ostree_object_name_serialize (checksum, objtype); + ostree_object_name_deserialize (key, &checksum, &objtype); if (g_hash_table_lookup_extended (data->reachable, key, NULL, NULL)) reachable = TRUE; @@ -276,17 +276,14 @@ repo_prune_internal (OstreeRepo *self, GLNX_HASH_TABLE_FOREACH_KV (objects, GVariant*, serialized_key, GVariant*, objdata) { - const char *checksum; - OstreeObjectType objtype; gboolean is_loose; - ostree_object_name_deserialize (serialized_key, &checksum, &objtype); g_variant_get_child (objdata, 0, "b", &is_loose); if (!is_loose) continue; - if (!maybe_prune_loose_object (&data, options->flags, checksum, objtype, + if (!maybe_prune_loose_object (&data, options->flags, serialized_key, cancellable, error)) return FALSE; } From 947acbf178152f860d3d8f9896b2ce51645419f6 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 19 Nov 2021 10:44:03 -0500 Subject: [PATCH 29/30] app: Only remount /sysroot if needed We should only try to remount `/sysroot` if we're actually handling the sysroot repo and the repo isn't writable. We already have public APIs to check each of those, so let's use them. Closes: #2485 --- src/ostree/ot-main.c | 58 +++++++++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index bbaba8b0..849a74ef 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -238,7 +238,7 @@ ostree_run (int argc, return 0; } -/* Process a --repo arg; used below, and for the remote builtins */ +/* Process a --repo arg. */ static OstreeRepo * parse_repo_option (GOptionContext *context, const char *repo_path, @@ -248,19 +248,6 @@ parse_repo_option (GOptionContext *context, { g_autoptr(OstreeRepo) repo = NULL; - /* This is a bit of a brutal hack; we set up a mount - * namespace if it appears that we may need it. It'd - * be better to do this more precisely in the future. - */ - gboolean setup_ns = FALSE; - if (!maybe_setup_mount_namespace (&setup_ns, error)) - return FALSE; - if (setup_ns) - { - if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) - return glnx_null_throw_errno_prefix (error, "Remounting /sysroot read-write"); - } - if (repo_path == NULL) { g_autoptr(GError) local_error = NULL; @@ -300,6 +287,43 @@ parse_repo_option (GOptionContext *context, return g_steal_pointer (&repo); } +/* Process a --repo arg, determining if we should remount /sysroot; used below, and for the remote builtins */ +static OstreeRepo * +parse_repo_option_and_maybe_remount (GOptionContext *context, + const char *repo_path, + gboolean skip_repo_open, + GCancellable *cancellable, + GError **error) +{ + g_autoptr(OstreeRepo) repo = parse_repo_option (context, repo_path, skip_repo_open, cancellable, error); + if (!repo) + return NULL; + + /* This is a bit of a brutal hack; we set up a mount + * namespace if it appears that we may need it. It'd + * be better to do this more precisely in the future. + */ + if (ostree_repo_is_system (repo) && !ostree_repo_is_writable (repo, NULL)) + { + gboolean setup_ns = FALSE; + if (!maybe_setup_mount_namespace (&setup_ns, error)) + return FALSE; + if (setup_ns) + { + if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0) + return glnx_null_throw_errno_prefix (error, "Remounting /sysroot read-write"); + + /* Reload the repo so it's actually writable. */ + g_clear_pointer (&repo, g_object_unref); + repo = parse_repo_option (context, repo_path, skip_repo_open, cancellable, error); + if (!repo) + return NULL; + } + } + + return g_steal_pointer (&repo); +} + /* Used by the remote builtins which are special in taking --sysroot or --repo */ gboolean ostree_parse_sysroot_or_repo_option (GOptionContext *context, @@ -323,7 +347,7 @@ ostree_parse_sysroot_or_repo_option (GOptionContext *context, } else { - repo = parse_repo_option (context, repo_path, FALSE, cancellable, error); + repo = parse_repo_option_and_maybe_remount (context, repo_path, FALSE, cancellable, error); if (!repo) return FALSE; } @@ -424,8 +448,8 @@ ostree_option_context_parse (GOptionContext *context, if (!(flags & OSTREE_BUILTIN_FLAG_NO_REPO)) { - repo = parse_repo_option (context, opt_repo, (flags & OSTREE_BUILTIN_FLAG_NO_CHECK) > 0, - cancellable, error); + repo = parse_repo_option_and_maybe_remount (context, opt_repo, (flags & OSTREE_BUILTIN_FLAG_NO_CHECK) > 0, + cancellable, error); if (!repo) return FALSE; } From f1155c8d283c3c85d74d5e1050b0dcf8198f750a Mon Sep 17 00:00:00 2001 From: Luca BRUNO Date: Tue, 23 Nov 2021 10:13:32 +0000 Subject: [PATCH 30/30] Release 2021.6 --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 554cffb2..ff174da8 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ m4_define([year_version], [2021]) m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux])