From 03a6466888985b7cece7ab2c715d78d36120faa662d1f47dc541aeda9d5da317 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Thu, 18 Feb 2021 10:03:25 +0000 Subject: [PATCH] Accepting request 873316 from home:jirislaby:branches:Base:System - Fix grub's requoted kernel parameters (bsc#1181111) + 0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch + 0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch Linux kernel >= v5.2~rc1, it is possible to get module OBS-URL: https://build.opensuse.org/request/show/873316 OBS-URL: https://build.opensuse.org/package/show/Base:System/kmod?expand=0&rev=184 --- ...evamp-kcmdline-parsing-into-a-state-.patch | 143 ++++++++++ ...-re-quote-option-from-kernel-cmdline.patch | 263 ++++++++++++++++++ kmod.changes | 9 +- kmod.spec | 2 + 4 files changed, 416 insertions(+), 1 deletion(-) create mode 100644 0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch create mode 100644 0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch diff --git a/0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch b/0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch new file mode 100644 index 0000000..e3b1985 --- /dev/null +++ b/0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch @@ -0,0 +1,143 @@ +From: Lucas De Marchi +Date: Fri, 12 Feb 2021 01:45:21 -0800 +Subject: libkmod-config: revamp kcmdline parsing into a state machine +Git-repo: git://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git +Git-commit: 01ed9af61e239b40514edf527ac87c79377266ac +Patch-mainline: v29 +References: bsc#1181111 + +The handling of spaces and quotes is becoming hard to maintain. Convert +the parser into a state machine so we can check all the states. This +should make it easier to fix a corner case we have right now: +The kernel also accepts a quote before the module name instead of the +value. But this additional is left for later. This is purely an +algorithm change with no behavior change. + +Tested-by: Jessica Yu +Signed-off-by: Jiri Slaby +--- + libkmod/libkmod-config.c | 86 ++++++++++++++++++++++++---------------- + 1 file changed, 52 insertions(+), 34 deletions(-) + +diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c +index 971f20b8a352..d3cd10d42a10 100644 +--- a/libkmod/libkmod-config.c ++++ b/libkmod/libkmod-config.c +@@ -499,7 +499,14 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + char buf[KCMD_LINE_SIZE]; + int fd, err; + char *p, *modname, *param = NULL, *value = NULL; +- bool is_quoted = false, is_module = true; ++ bool is_quoted = false, iter = true; ++ enum state { ++ STATE_IGNORE, ++ STATE_MODNAME, ++ STATE_PARAM, ++ STATE_VALUE, ++ STATE_COMPLETE, ++ } state; + + fd = open("/proc/cmdline", O_RDONLY|O_CLOEXEC); + if (fd < 0) { +@@ -516,54 +523,65 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + return err; + } + +- for (p = buf, modname = buf; *p != '\0' && *p != '\n'; p++) { +- if (*p == '"') { ++ state = STATE_MODNAME; ++ for (p = buf, modname = buf; iter; p++) { ++ switch (*p) { ++ case '"': + is_quoted = !is_quoted; +- +- if (is_quoted) { +- /* don't consider a module until closing quotes */ +- is_module = false; +- } else if (param != NULL && value != NULL) { ++ break; ++ case '\0': ++ case '\n': ++ /* Stop iterating on new chars */ ++ iter = false; ++ /* fall-through */ ++ case ' ': ++ if (is_quoted && state == STATE_VALUE) { ++ /* no state change*/; ++ } else if (is_quoted) { ++ /* spaces are only allowed in the value part */ ++ state = STATE_IGNORE; ++ } else if (state == STATE_VALUE || state == STATE_PARAM) { ++ *p = '\0'; ++ state = STATE_COMPLETE; ++ } else { + /* +- * If we are indeed expecting a value and +- * closing quotes, then this can be considered +- * a valid option for a module ++ * go to next option, ignoring any possible ++ * partial match we have + */ +- is_module = true; ++ modname = p + 1; ++ state = STATE_MODNAME; + } +- +- continue; +- } +- if (is_quoted) +- continue; +- +- switch (*p) { +- case ' ': +- *p = '\0'; +- if (is_module) +- kcmdline_parse_result(config, modname, param, value); +- param = value = NULL; +- modname = p + 1; +- is_module = true; + break; + case '.': +- if (param == NULL) { ++ if (state == STATE_MODNAME) { + *p = '\0'; + param = p + 1; ++ state = STATE_PARAM; ++ } else if (state == STATE_PARAM) { ++ state = STATE_IGNORE; + } + break; + case '=': +- if (param != NULL) ++ if (state == STATE_PARAM) { ++ /* ++ * Don't set *p to '\0': the value var shadows ++ * param ++ */ + value = p + 1; +- else +- is_module = false; ++ state = STATE_VALUE; ++ } else if (state == STATE_MODNAME) { ++ state = STATE_IGNORE; ++ } + break; + } +- } + +- *p = '\0'; +- if (is_module) +- kcmdline_parse_result(config, modname, param, value); ++ if (state == STATE_COMPLETE) { ++ kcmdline_parse_result(config, modname, param, value); ++ /* start over on next iteration */ ++ modname = p + 1; ++ state = STATE_MODNAME; ++ } ++ } + + return 0; + } +-- +2.30.1 + diff --git a/0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch b/0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch new file mode 100644 index 0000000..f9390c8 --- /dev/null +++ b/0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch @@ -0,0 +1,263 @@ +From: Lucas De Marchi +Date: Fri, 12 Feb 2021 01:45:22 -0800 +Subject: libkmod-config: re-quote option from kernel cmdline +Git-repo: git://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git +Git-commit: d3a1fe67b64cad103ff4f93dfd9f2cf19cab09ba +Patch-mainline: v29 +References: bsc#1181111 + +It was reported that grub mangles the kernel cmdline. It turns + + acpi_cpufreq.dyndbg="file drivers/cpufreq/acpi-cpufreq.c +mpf" + + into + + "acpi_cpufreq.dyndbg=file drivers/cpufreq/acpi-cpufreq.c +mpf" + +However, even though we could blame grub for doing that, the kernel +happily accepts and re-quotes it when the module is built-in. +So, it's better if kmod also understands it this way and does the same. + +Here we basically add additional code to un-mangle it, moving the quote +in way that is acceptable to pass through init_module(). Note that the +interface [f]init_module() gives us mandates the quote to be part of the +value: the module name is not passed and the options are separated by +space. + +Reported-by: Jiri Slaby +Tested-by: Jessica Yu +Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10 +Signed-off-by: Jiri Slaby +--- + libkmod/libkmod-config.c | 36 ++++++++++++- + .../module-param-kcmdline7/correct.txt | 5 ++ + .../module-param-kcmdline7/correct.txt | 5 ++ + .../module-param-kcmdline7/proc/cmdline | 1 + + .../module-param-kcmdline7/proc/cmdline | 1 + + .../module-param-kcmdline8/correct.txt | 5 ++ + .../module-param-kcmdline7/correct.txt | 5 ++ + .../module-param-kcmdline7/proc/cmdline | 1 + + .../module-param-kcmdline8/proc/cmdline | 1 + + testsuite/test-modprobe.c | 50 +++++++++++++++++++ + 10 files changed, 109 insertions(+), 1 deletion(-) + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/correct.txt + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/correct.txt + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/proc/cmdline + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/proc/cmdline + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/correct.txt + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/correct.txt + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/proc/cmdline + create mode 100644 testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/proc/cmdline + +diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c +index d3cd10d42a10..2873f061dc9e 100644 +--- a/libkmod/libkmod-config.c ++++ b/libkmod/libkmod-config.c +@@ -498,7 +498,7 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + { + char buf[KCMD_LINE_SIZE]; + int fd, err; +- char *p, *modname, *param = NULL, *value = NULL; ++ char *p, *p_quote_start, *modname, *param = NULL, *value = NULL; + bool is_quoted = false, iter = true; + enum state { + STATE_IGNORE, +@@ -524,10 +524,23 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + } + + state = STATE_MODNAME; ++ p_quote_start = NULL; + for (p = buf, modname = buf; iter; p++) { + switch (*p) { + case '"': + is_quoted = !is_quoted; ++ ++ /* ++ * only allow starting quote as first char when looking ++ * for a modname: anything else is considered ill-formed ++ */ ++ if (is_quoted && state == STATE_MODNAME && p == modname) { ++ p_quote_start = p; ++ modname = p + 1; ++ } else if (state != STATE_VALUE) { ++ state = STATE_IGNORE; ++ } ++ + break; + case '\0': + case '\n': +@@ -550,6 +563,7 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + */ + modname = p + 1; + state = STATE_MODNAME; ++ p_quote_start = NULL; + } + break; + case '.': +@@ -576,10 +590,30 @@ static int kmod_config_parse_kcmdline(struct kmod_config *config) + } + + if (state == STATE_COMPLETE) { ++ /* ++ * We may need to re-quote to unmangle what the ++ * bootloader passed. Example: grub passes the option as ++ * "parport.dyndbg=file drivers/parport/ieee1284_ops.c +mpf" ++ * instead of ++ * parport.dyndbg="file drivers/parport/ieee1284_ops.c +mpf" ++ */ ++ if (p_quote_start && p_quote_start < modname) { ++ /* ++ * p_quote_start ++ * | ++ * |modname param value ++ * || | | ++ * vv v v ++ * "parport\0dyndbg=file drivers/parport/ieee1284_ops.c +mpf" */ ++ memmove(p_quote_start, modname, value - modname); ++ value--; modname--; param--; ++ *value = '"'; ++ } + kcmdline_parse_result(config, modname, param, value); + /* start over on next iteration */ + modname = p + 1; + state = STATE_MODNAME; ++ p_quote_start = NULL; + } + } + +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/correct.txt +new file mode 100644 +index 000000000000..d80da6d802af +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/correct.txt +@@ -0,0 +1,5 @@ ++options psmouse foo ++options parport dyndbg="file drivers/parport/ieee1284_ops.c +mpf" ++ ++# End of configuration files. Dumping indexes now: ++ +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/correct.txt +new file mode 100644 +index 000000000000..d80da6d802af +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/correct.txt +@@ -0,0 +1,5 @@ ++options psmouse foo ++options parport dyndbg="file drivers/parport/ieee1284_ops.c +mpf" ++ ++# End of configuration files. Dumping indexes now: ++ +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/proc/cmdline +new file mode 100644 +index 000000000000..86f9052394a0 +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/module-param-kcmdline7/proc/cmdline +@@ -0,0 +1 @@ ++psmouse.foo parport.dyndbg="file drivers/parport/ieee1284_ops.c +mpf" quiet rw +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/proc/cmdline +new file mode 100644 +index 000000000000..86f9052394a0 +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline7/proc/cmdline +@@ -0,0 +1 @@ ++psmouse.foo parport.dyndbg="file drivers/parport/ieee1284_ops.c +mpf" quiet rw +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/correct.txt +new file mode 100644 +index 000000000000..d80da6d802af +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/correct.txt +@@ -0,0 +1,5 @@ ++options psmouse foo ++options parport dyndbg="file drivers/parport/ieee1284_ops.c +mpf" ++ ++# End of configuration files. Dumping indexes now: ++ +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/correct.txt b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/correct.txt +new file mode 100644 +index 000000000000..d80da6d802af +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/correct.txt +@@ -0,0 +1,5 @@ ++options psmouse foo ++options parport dyndbg="file drivers/parport/ieee1284_ops.c +mpf" ++ ++# End of configuration files. Dumping indexes now: ++ +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/proc/cmdline +new file mode 100644 +index 000000000000..86f9052394a0 +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/module-param-kcmdline7/proc/cmdline +@@ -0,0 +1 @@ ++psmouse.foo parport.dyndbg="file drivers/parport/ieee1284_ops.c +mpf" quiet rw +diff --git a/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/proc/cmdline b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/proc/cmdline +new file mode 100644 +index 000000000000..eab04adbd5f8 +--- /dev/null ++++ b/testsuite/rootfs-pristine/test-modprobe/module-param-kcmdline8/proc/cmdline +@@ -0,0 +1 @@ ++psmouse.foo "parport.dyndbg=file drivers/parport/ieee1284_ops.c +mpf" quiet rw +diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c +index f6bed8bd3487..dbc54f37b377 100644 +--- a/testsuite/test-modprobe.c ++++ b/testsuite/test-modprobe.c +@@ -359,6 +359,56 @@ DEFINE_TEST(modprobe_param_kcmdline6, + ); + + ++static noreturn int modprobe_param_kcmdline7(const struct test *t) ++{ ++ const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe"; ++ const char *const args[] = { ++ progname, ++ "-c", ++ NULL, ++ }; ++ ++ test_spawn_prog(progname, args); ++ exit(EXIT_FAILURE); ++} ++DEFINE_TEST(modprobe_param_kcmdline7, ++ .description = "check if dots on other parts of kcmdline don't confuse our parser", ++ .config = { ++ [TC_UNAME_R] = "4.4.4", ++ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline7", ++ }, ++ .output = { ++ .out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline7/correct.txt", ++ }, ++ .modules_loaded = "", ++ ); ++ ++ ++static noreturn int modprobe_param_kcmdline8(const struct test *t) ++{ ++ const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe"; ++ const char *const args[] = { ++ progname, ++ "-c", ++ NULL, ++ }; ++ ++ test_spawn_prog(progname, args); ++ exit(EXIT_FAILURE); ++} ++DEFINE_TEST(modprobe_param_kcmdline8, ++ .description = "check if dots on other parts of kcmdline don't confuse our parser", ++ .config = { ++ [TC_UNAME_R] = "4.4.4", ++ [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline8", ++ }, ++ .output = { ++ .out = TESTSUITE_ROOTFS "test-modprobe/module-param-kcmdline8/correct.txt", ++ }, ++ .modules_loaded = "", ++ ); ++ ++ + static noreturn int modprobe_force(const struct test *t) + { + const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe"; +-- +2.30.1 + diff --git a/kmod.changes b/kmod.changes index 4bb8f2e..165f5f7 100644 --- a/kmod.changes +++ b/kmod.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Feb 18 08:19:01 UTC 2021 - Jiri Slaby + +- Fix grub's requoted kernel parameters (bsc#1181111) + + 0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch + + 0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch + ------------------------------------------------------------------- Thu Feb 4 08:55:08 UTC 2021 - Michal Suchanek @@ -72,7 +79,7 @@ Wed May 27 23:31:52 UTC 2020 - Jan Engelhardt * Use PKCS#7 instead of CMS for parsing module signature to be compatible with LibreSSL and OpenSSL < 1.1.0. * Teach modinfo to parse modules.builtin.modinfo. When using - Linux kernel >= v5.2~rc1, it is possible to get module + Linux kernel >= v5.2~rc1, it is possible to get module information from this new file. ------------------------------------------------------------------- diff --git a/kmod.spec b/kmod.spec index 144674b..a0f8b3e 100644 --- a/kmod.spec +++ b/kmod.spec @@ -38,6 +38,8 @@ Patch6: 0012-modprobe-print-unsupported-status.patch Patch7: usr-lib-modprobe.patch Patch8: no-stylesheet-download.patch Patch9: 0001-Fix-modinfo-F-always-shows-name-for-built-ins.patch +Patch10: 0001-libkmod-config-revamp-kcmdline-parsing-into-a-state-.patch +Patch11: 0002-libkmod-config-re-quote-option-from-kernel-cmdline.patch BuildRequires: autoconf BuildRequires: automake BuildRequires: docbook5-xsl-stylesheets