From 20b8cb174c3e3aa482a856f0a4323849ce7b05de Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 23 Aug 2023 16:06:23 -0400 Subject: [PATCH] Move prepare-root karg helpers into otcore, add unit tests Add long overdue unit testing coverage for this, which at least slightly closes out the android boot CI gap. Actually, this *copies* the karg parsing code into otcore because it now uses glib, which we're not yet using in the static prepare-root. It's pretty tempting to drop support for the static prepare root entirely. But for now we'll live with some code duplication. --- Makefile-otcore.am | 1 + src/libostree/ostree-impl-system-generator.c | 4 +- src/libotcore/otcore-prepare-root.c | 108 +++++++++++++++++++ src/libotcore/otcore.h | 3 + src/switchroot/ostree-prepare-root.c | 54 +--------- tests/test-otcore.c | 51 +++++++++ 6 files changed, 170 insertions(+), 51 deletions(-) create mode 100644 src/libotcore/otcore-prepare-root.c diff --git a/Makefile-otcore.am b/Makefile-otcore.am index 8accf858..8252ead0 100644 --- a/Makefile-otcore.am +++ b/Makefile-otcore.am @@ -18,6 +18,7 @@ noinst_LTLIBRARIES += libotcore.la libotcore_la_SOURCES = \ src/libotcore/otcore.h \ src/libotcore/otcore-ed25519-verify.c \ + src/libotcore/otcore-prepare-root.c \ $(NULL) libotcore_la_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/libglnx -I$(srcdir)/src/libotutil -DLOCALEDIR=\"$(datadir)/locale\" $(OT_INTERNAL_GIO_UNIX_CFLAGS) $(OT_INTERNAL_GPGME_CFLAGS) $(OT_DEP_CRYPTO_LIBS) $(LIBSYSTEMD_CFLAGS) diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c index c5db4aa3..ad785eb9 100644 --- a/src/libostree/ostree-impl-system-generator.c +++ b/src/libostree/ostree-impl-system-generator.c @@ -34,7 +34,7 @@ #include "ostree-core-private.h" #include "ostree-mount-util.h" #include "ostree-sysroot-private.h" -#include "ostree.h" +#include "otcore.h" #ifdef HAVE_LIBMOUNT typedef FILE OtLibMountFile; @@ -260,7 +260,7 @@ _ostree_impl_system_generator (const char *normal_dir, const char *early_dir, co * exit so that we don't error, but at the same time work where switchroot * is PID 1 (and so hasn't created /run/ostree-booted). */ - g_autofree char *ostree_cmdline = find_proc_cmdline_key (cmdline, "ostree"); + g_autofree char *ostree_cmdline = otcore_find_proc_cmdline_key (cmdline, "ostree"); if (!ostree_cmdline) return TRUE; diff --git a/src/libotcore/otcore-prepare-root.c b/src/libotcore/otcore-prepare-root.c new file mode 100644 index 00000000..f3b1a086 --- /dev/null +++ b/src/libotcore/otcore-prepare-root.c @@ -0,0 +1,108 @@ +/* + * SPDX-License-Identifier: LGPL-2.0+ + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see . + */ + +#include "config.h" + +#include "otcore.h" + +static bool +proc_cmdline_has_key_starting_with (const char *cmdline, const char *key) +{ + for (const char *iter = cmdline; iter;) + { + if (g_str_has_prefix (iter, key)) + return true; + + iter = strchr (iter, ' '); + if (iter == NULL) + return false; + + iter += strspn (iter, " "); + } + + return false; +} + +// Parse a kernel cmdline to find the provided key. +// TODO: Deduplicate this with the kernel argument code from libostree.so +char * +otcore_find_proc_cmdline_key (const char *cmdline, const char *key) +{ + const size_t key_len = strlen (key); + for (const char *iter = cmdline; iter;) + { + const char *next = strchr (iter, ' '); + if (strncmp (iter, key, key_len) == 0 && iter[key_len] == '=') + { + const char *start = iter + key_len + 1; + if (next) + return strndup (start, next - start); + + return strdup (start); + } + + if (next) + next += strspn (next, " "); + + iter = next; + } + + return NULL; +} + +// Find the target OSTree root filesystem from parsing the provided kernel commandline. +// If none is found, @out_target will be set to NULL, and the function will return successfully. +// +// If invalid data is found, @error will be set. +gboolean +otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error) +{ + g_assert (cmdline); + g_assert (out_target && *out_target == NULL); + static const char slot_a[] = "/ostree/root.a"; + static const char slot_b[] = "/ostree/root.b"; + + // First, handle the Android boot case + g_autofree char *slot_suffix = otcore_find_proc_cmdline_key (cmdline, "androidboot.slot_suffix"); + if (slot_suffix) + { + if (strcmp (slot_suffix, "_a") == 0) + { + *out_target = strdup (slot_a); + return TRUE; + } + else if (strcmp (slot_suffix, "_b") == 0) + { + *out_target = strdup (slot_b); + return TRUE; + } + return glnx_throw (error, "androidboot.slot_suffix invalid: %s", slot_suffix); + } + + /* Non-A/B androidboot: + * https://source.android.com/docs/core/ota/nonab + */ + if (proc_cmdline_has_key_starting_with (cmdline, "androidboot.")) + { + *out_target = strdup (slot_a); + return TRUE; + } + + // Otherwise, fall back to the default `ostree=` kernel cmdline + *out_target = otcore_find_proc_cmdline_key (cmdline, "ostree"); + return TRUE; +} diff --git a/src/libotcore/otcore.h b/src/libotcore/otcore.h index a5bdb8f9..d41758e1 100644 --- a/src/libotcore/otcore.h +++ b/src/libotcore/otcore.h @@ -43,6 +43,9 @@ bool otcore_ed25519_init (void); gboolean otcore_validate_ed25519_signature (GBytes *data, GBytes *pubkey, GBytes *signature, bool *out_valid, GError **error); +char *otcore_find_proc_cmdline_key (const char *cmdline, const char *key); +gboolean otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error); + // Our directory with transient state (eventually /run/ostree-booted should be a link to // /run/ostree/booted) #define OTCORE_RUN_OSTREE "/run/ostree" diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 155189b9..62ea5617 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -150,53 +150,6 @@ sysroot_is_configured_ro (const char *sysroot) return g_key_file_get_boolean (repo_config, SYSROOT_KEY, READONLY_KEY, NULL); } -static inline char * -get_aboot_root_slot (const char *slot_suffix) -{ - if (strcmp (slot_suffix, "_a") == 0) - return strdup ("/ostree/root.a"); - else if (strcmp (slot_suffix, "_b") == 0) - return strdup ("/ostree/root.b"); - - errx (EXIT_FAILURE, "androidboot.slot_suffix invalid: %s", slot_suffix); - - return NULL; -} - -static bool -proc_cmdline_has_key_starting_with (const char *cmdline, const char *key) -{ - for (const char *iter = cmdline; iter;) - { - if (g_str_has_prefix (iter, key)) - return true; - - iter = strchr (iter, ' '); - if (iter == NULL) - return false; - - iter += strspn (iter, " "); - } - - return false; -} - -static inline char * -get_ostree_target (const char *cmdline) -{ - autofree char *slot_suffix = find_proc_cmdline_key (cmdline, "androidboot.slot_suffix"); - if (slot_suffix) - return get_aboot_root_slot (slot_suffix); - - /* Non-A/B androidboot: - * https://source.android.com/docs/core/ota/nonab - */ - if (proc_cmdline_has_key_starting_with (cmdline, "androidboot.")) - return strdup ("/ostree/root.a"); - - return find_proc_cmdline_key (cmdline, "ostree"); -} - static char * resolve_deploy_path (const char *root_mountpoint) { @@ -207,9 +160,12 @@ resolve_deploy_path (const char *root_mountpoint) if (!kernel_cmdline) errx (EXIT_FAILURE, "Failed to read kernel cmdline"); - g_autofree char *ostree_target = get_ostree_target (kernel_cmdline); + g_autoptr (GError) error = NULL; + g_autofree char *ostree_target = NULL; + if (!otcore_get_ostree_target (kernel_cmdline, &ostree_target, &error)) + errx (EXIT_FAILURE, "Failed to determine ostree target: %s", error->message); if (!ostree_target) - errx (EXIT_FAILURE, "No ostree target"); + errx (EXIT_FAILURE, "No ostree target found"); if (snprintf (destpath, sizeof (destpath), "%s/%s", root_mountpoint, ostree_target) < 0) err (EXIT_FAILURE, "failed to assemble ostree target path"); diff --git a/tests/test-otcore.c b/tests/test-otcore.c index cac42096..03cfb6c8 100644 --- a/tests/test-otcore.c +++ b/tests/test-otcore.c @@ -31,11 +31,62 @@ test_ed25519 (void) g_clear_error (&error); } +static void +test_prepare_root_cmdline (void) +{ + g_autoptr (GError) error = NULL; + g_autofree char *target = NULL; + + static const char *notfound_cases[] + = { "", "foo", "foo=bar baz sometest", "xostree foo", "xostree=blah bar", NULL }; + for (const char **iter = notfound_cases; iter && *iter; iter++) + { + const char *tcase = *iter; + g_assert (otcore_get_ostree_target (tcase, &target, &error)); + g_assert_no_error (error); + g_assert (target == NULL); + } + + // Test the default ostree= + g_assert (otcore_get_ostree_target ("blah baz=blah ostree=/foo/bar somearg", &target, &error)); + g_assert_no_error (error); + g_assert_cmpstr (target, ==, "/foo/bar"); + free (g_steal_pointer (&target)); + + // Test android boot + g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_b somearg", &target, + &error)); + g_assert_no_error (error); + g_assert_cmpstr (target, ==, "/ostree/root.b"); + free (g_steal_pointer (&target)); + + g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_a somearg", &target, + &error)); + g_assert_no_error (error); + g_assert_cmpstr (target, ==, "/ostree/root.a"); + free (g_steal_pointer (&target)); + + // And an expected failure to parse a "c" suffix + g_assert (!otcore_get_ostree_target ("blah baz=blah androidboot.slot_suffix=_c somearg", &target, + &error)); + g_assert (error); + g_assert (target == NULL); + g_clear_error (&error); + + // And non-A/B androidboot + g_assert (otcore_get_ostree_target ("blah baz=blah androidboot.somethingelse somearg", &target, + &error)); + g_assert_no_error (error); + g_assert_cmpstr (target, ==, "/ostree/root.a"); + free (g_steal_pointer (&target)); +} + int main (int argc, char **argv) { g_test_init (&argc, &argv, NULL); otcore_ed25519_init (); g_test_add_func ("/ed25519", test_ed25519); + g_test_add_func ("/prepare-root-cmdline", test_prepare_root_cmdline); return g_test_run (); } -- 2.30.2