Accepting request 873347 from Base:System

- Fix grub's requoted kernel parameters (bsc#1181111)

OBS-URL: https://build.opensuse.org/request/show/873347
OBS-URL: https://build.opensuse.org/package/show/openSUSE:Factory/kmod?expand=0&rev=61
This commit is contained in:
Richard Brown 2021-02-22 13:23:04 +00:00 committed by Git OBS Bridge
commit da0e210225
4 changed files with 420 additions and 5 deletions

View File

@ -0,0 +1,143 @@
From: Lucas De Marchi <lucas.demarchi@intel.com>
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 <jeyu@kernel.org>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
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

View File

@ -0,0 +1,263 @@
From: Lucas De Marchi <lucas.demarchi@intel.com>
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 <jirislaby@kernel.org>
Tested-by: Jessica Yu <jeyu@kernel.org>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
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

View File

@ -1,10 +1,17 @@
-------------------------------------------------------------------
Thu Feb 18 08:19:01 UTC 2021 - Jiri Slaby <jslaby@suse.cz>
- 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 <msuchanek@suse.de>
- Fix tests to not test disabled features. Disable zstd again.
+ kmod-populate-modules-Use-more-bash-more-quotes.patch
+ kmod-testsuite-compress-modules-if-feature-is-enabled.patch
+ kmod-also-test-xz-compression.patch
* kmod-populate-modules-Use-more-bash-more-quotes.patch
* kmod-testsuite-compress-modules-if-feature-is-enabled.patch
* kmod-also-test-xz-compression.patch
-------------------------------------------------------------------
Fri Jan 29 11:54:30 UTC 2021 - Dominique Leuenberger <dimstar@opensuse.org>
@ -36,7 +43,7 @@ Tue Jan 12 16:04:41 UTC 2021 - Michal Suchanek <msuchanek@suse.de>
- Update usr-lib-modprobe.patch to upstream submission (boo#1180821).
- Require libxslt-tools for xsltproc and use local stylesheet.
+ no-stylesheet-download.patch
* no-stylesheet-download.patch
-------------------------------------------------------------------
Fri Nov 6 11:41:50 UTC 2020 - Jan Engelhardt <jengelh@inai.de>
@ -72,7 +79,7 @@ Wed May 27 23:31:52 UTC 2020 - Jan Engelhardt <jengelh@inai.de>
* 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.
-------------------------------------------------------------------

View File

@ -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