From 44dbf13f2a95f1eea96812d35c01ce317fdb834b Mon Sep 17 00:00:00 2001 From: Joseph Marrero Corchado Date: Sun, 12 Apr 2026 22:20:38 -0400 Subject: [PATCH] kargs: Fix crash on quoted values in /proc/cmdline ostree_kernel_args_append_proc_cmdline() crashes with SIGABRT when /proc/cmdline contains quoted kernel arguments with spaces (e.g. "testparam=value with spaces"). This was reported by Arch Linux maintainers when building ostree 2026.1 and reproduced on 2025.7. Root cause: commit abc7d5b9 ("kargs: parse spaces in kargs input and keep quotes", March 2024) added split_kernel_args(), a quote-aware string splitter, and updated ostree_kernel_args_append() and three other functions to use it. However, it did not update ostree_kernel_args_append_proc_cmdline(), which still used a naive g_strsplit(" ") to tokenize /proc/cmdline. When the cmdline contained quoted values with spaces, g_strsplit broke them into fragments with unterminated quotes. These fragments were then passed to ostree_kernel_args_append() -> split_kernel_args(), which hit a g_assert_false(quoted) and aborted. The bug was latent since v2024.5 because the test-admin-deploy-karg.sh and test-admin-instutil-set-kargs.sh tests read the real /proc/cmdline from the build host. CI environments never had quoted values in their cmdline, so the crash never triggered. The Arch build server recently acquired a quoted value (likely from a kernel or GRUB config change), exposing the bug. Additionally, bootloaders may reformat quotes: what rpm-ostree stores as testparam="value with spaces" appears in /proc/cmdline as "testparam=value with spaces" (GRUB wraps the entire token in quotes instead of just the value). Fix this with two changes: 1. Replace g_strsplit() with split_kernel_args() in ostree_kernel_args_append_proc_cmdline() so the initial tokenization of /proc/cmdline is quote-aware. Individual args passed to ostree_kernel_args_append() are now already properly split. 2. Replace g_assert_false(quoted) in split_kernel_args() with a g_debug() warning that continues execution, treating the remainder as a single token. Since we cannot control what bootloaders put in /proc/cmdline, a hard abort on unterminated quotes is never appropriate. Add a unit test (test_kargs_quoted_cmdline) covering standard quoted values, GRUB-reformatted quoting, and multiple quoted args. Add a kola destructive test (kargs-proc-cmdline-quoted.sh) that injects a quoted karg via rpm-ostree, reboots, and verifies --karg-proc-cmdline and --import-proc-cmdline succeed without crashing. Assisted-by: OpenCode (Claude claude-opus-4-6) Signed-off-by: Joseph Marrero Corchado --- src/libostree/ostree-kernel-args.c | 15 +++--- .../destructive/kargs-proc-cmdline-quoted.sh | 44 ++++++++++++++++ tests/test-kargs.c | 50 +++++++++++++++++++ 3 files changed, 102 insertions(+), 7 deletions(-) create mode 100755 tests/kolainst/destructive/kargs-proc-cmdline-quoted.sh diff --git a/src/libostree/ostree-kernel-args.c b/src/libostree/ostree-kernel-args.c index 056b3fa926..3b33f16100 100644 --- a/src/libostree/ostree-kernel-args.c +++ b/src/libostree/ostree-kernel-args.c @@ -188,13 +188,14 @@ split_kernel_args (const char *str) } } - // Add the last slice - if (!quoted) - g_ptr_array_add (strv, g_strndup (start, str + len - start)); - else + // Add the last slice if non-empty. If there's an unterminated quote + // (e.g. from bootloader-reformatted /proc/cmdline), treat the + // remainder as a single token rather than aborting. + if (str + len > start) { - g_debug ("Missing terminating quote in '%s'.\n", str); - g_assert_false (quoted); + if (quoted) + g_debug ("Missing terminating quote in '%s', treating remainder as single token.\n", str); + g_ptr_array_add (strv, g_strndup (start, str + len - start)); } g_ptr_array_add (strv, NULL); @@ -681,7 +682,7 @@ ostree_kernel_args_append_proc_cmdline (OstreeKernelArgs *kargs, GCancellable *c g_strchomp (proc_cmdline); - proc_cmdline_args = g_strsplit (proc_cmdline, " ", -1); + proc_cmdline_args = split_kernel_args (proc_cmdline); ostree_kernel_args_append_argv_filtered (kargs, proc_cmdline_args, filtered_prefixes); return TRUE; diff --git a/tests/kolainst/destructive/kargs-proc-cmdline-quoted.sh b/tests/kolainst/destructive/kargs-proc-cmdline-quoted.sh new file mode 100755 index 0000000000..32d8d1f907 --- /dev/null +++ b/tests/kolainst/destructive/kargs-proc-cmdline-quoted.sh @@ -0,0 +1,44 @@ +#!/bin/bash + +# Verify ostree handles quoted kernel args with spaces in /proc/cmdline. +# +# Regression test for a bug introduced in abc7d5b9 where +# ostree_kernel_args_append_proc_cmdline() used a naive g_strsplit(" ") +# to tokenize /proc/cmdline, then passed the fragments to +# ostree_kernel_args_append() which calls the quote-aware +# split_kernel_args(). Fragments with unterminated quotes caused a +# g_assert_false(quoted) abort. +# +# The bootloader (GRUB) may reformat quoted kargs -- e.g. what +# rpm-ostree stores as testparam="value with spaces" can appear in +# /proc/cmdline as "testparam=value with spaces" (quotes wrapping +# the entire token). The parser must handle both forms. + +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in +"") + # Append a quoted karg containing spaces + rpm-ostree kargs --append='testparam="value with spaces"' + /tmp/autopkgtest-reboot "verify" + ;; +"verify") + # The karg should be present in /proc/cmdline (possibly reformatted + # by the bootloader, e.g. "testparam=value with spaces") + assert_file_has_content /proc/cmdline testparam + + # These commands read /proc/cmdline and previously crashed with: + # g_assert_false(quoted) in split_kernel_args() + ostree admin instutil set-kargs --import-proc-cmdline + echo "ok import-proc-cmdline with quoted kargs" + + host_commit=$(ostree admin status | sed -n 's/^.* \(.*\)\.0$/\1/p' | head -1) + ostree admin deploy --karg-proc-cmdline "${host_commit}" + echo "ok deploy --karg-proc-cmdline with quoted kargs" + ;; +*) + fatal "Unexpected AUTOPKGTEST_REBOOT_MARK=${AUTOPKGTEST_REBOOT_MARK}" + ;; +esac diff --git a/tests/test-kargs.c b/tests/test-kargs.c index 90f11e5f85..3573657ec4 100644 --- a/tests/test-kargs.c +++ b/tests/test-kargs.c @@ -278,6 +278,55 @@ test_kargs_append (void) g_assert_cmpint (7, ==, g_strv_length (kargs_list)); } +/* Regression test: parsing a /proc/cmdline-style string containing quoted + * kernel args with spaces must not crash. Commit abc7d5b9 added a + * quote-aware splitter (split_kernel_args) but ostree_kernel_args_append_proc_cmdline + * still used a naive g_strsplit(" ") that broke quoted tokens apart, causing a + * g_assert_false(quoted) abort. + * + * Bootloaders may reformat quotes -- e.g. GRUB turns testparam="value with spaces" + * into "testparam=value with spaces" in /proc/cmdline. Both forms must be handled. + */ +static void +test_kargs_quoted_cmdline (void) +{ + /* Test 1: Standard quoted value (as stored by rpm-ostree) */ + { + __attribute__ ((cleanup (ostree_kernel_args_cleanup))) OstreeKernelArgs *karg + = ostree_kernel_args_new (); + ostree_kernel_args_parse_append (karg, + "root=UUID=abc quiet testparam=\"value with spaces\" rw"); + g_assert (check_string_existance (karg, "root=UUID=abc")); + g_assert (check_string_existance (karg, "quiet")); + g_assert (check_string_existance (karg, "testparam=\"value with spaces\"")); + g_assert (check_string_existance (karg, "rw")); + } + + /* Test 2: GRUB-reformatted quoting (quotes wrapping entire token) + * /proc/cmdline may show: "testparam=value with spaces" */ + { + __attribute__ ((cleanup (ostree_kernel_args_cleanup))) OstreeKernelArgs *karg + = ostree_kernel_args_new (); + ostree_kernel_args_parse_append (karg, + "root=UUID=abc quiet \"testparam=value with spaces\" rw"); + g_assert (check_string_existance (karg, "root=UUID=abc")); + g_assert (check_string_existance (karg, "quiet")); + g_assert (check_string_existance (karg, "\"testparam=value with spaces\"")); + g_assert (check_string_existance (karg, "rw")); + } + + /* Test 3: Multiple quoted args */ + { + __attribute__ ((cleanup (ostree_kernel_args_cleanup))) OstreeKernelArgs *karg + = ostree_kernel_args_new (); + ostree_kernel_args_parse_append (karg, "foo=\"1 2\" bar=\"3 4\""); + g_assert (check_string_existance (karg, "foo=\"1 2\"")); + g_assert (check_string_existance (karg, "bar=\"3 4\"")); + g_auto (GStrv) kargs_list = ostree_kernel_args_to_strv (karg); + g_assert_cmpint (2, ==, g_strv_length (kargs_list)); + } +} + int main (int argc, char *argv[]) { @@ -286,5 +335,6 @@ main (int argc, char *argv[]) g_test_add_func ("/kargs/kargs_append", test_kargs_append); g_test_add_func ("/kargs/kargs_delete", test_kargs_delete); g_test_add_func ("/kargs/kargs_replace", test_kargs_replace); + g_test_add_func ("/kargs/kargs_quoted_cmdline", test_kargs_quoted_cmdline); return g_test_run (); }