kargs: Fix crash on quoted values in /proc/cmdline

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;

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

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 ();
 }