All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine
@ 2021-02-12  9:45 Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lucas De Marchi @ 2021-02-12  9:45 UTC (permalink / raw)
  To: linux-modules; +Cc: Jiri Slaby, Jessica Yu, Giovanni Gherdovich

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.
---
 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 971f20b..d3cd10d 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.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline
  2021-02-12  9:45 [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine Lucas De Marchi
@ 2021-02-12  9:45 ` Lucas De Marchi
  2021-02-15 15:28   ` Jessica Yu
  2021-02-12  9:45 ` [PATCH 3/4] testsuite: allow to re-use single function for tests Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 4/4] test-modprobe: share single function for kcmdline tests Lucas De Marchi
  2 siblings, 1 reply; 6+ messages in thread
From: Lucas De Marchi @ 2021-02-12  9:45 UTC (permalink / raw)
  To: linux-modules; +Cc: Jiri Slaby, Jessica Yu, Giovanni Gherdovich

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>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10
---
 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 d3cd10d..2873f06 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 0000000..d80da6d
--- /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 0000000..d80da6d
--- /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 0000000..86f9052
--- /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 0000000..86f9052
--- /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 0000000..d80da6d
--- /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 0000000..d80da6d
--- /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 0000000..86f9052
--- /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 0000000..eab04ad
--- /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 f6bed8b..dbc54f3 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.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] testsuite: allow to re-use single function for tests
  2021-02-12  9:45 [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
@ 2021-02-12  9:45 ` Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 4/4] test-modprobe: share single function for kcmdline tests Lucas De Marchi
  2 siblings, 0 replies; 6+ messages in thread
From: Lucas De Marchi @ 2021-02-12  9:45 UTC (permalink / raw)
  To: linux-modules; +Cc: Jiri Slaby, Jessica Yu, Giovanni Gherdovich

Add a new DEFINE_TEST_WITH_FUNC() that accepts the function
alongside the test name. This will allow us to share a single function
for different tests.
---
 testsuite/testsuite.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
index c74b648..44d1730 100644
--- a/testsuite/testsuite.h
+++ b/testsuite/testsuite.h
@@ -140,14 +140,16 @@ int test_run(const struct test *t);
 
 
 /* Test definitions */
-#define DEFINE_TEST(_name, ...) \
+#define DEFINE_TEST_WITH_FUNC(_name, _func, ...) \
 	static const struct test UNIQ(s##_name) \
 	__attribute__((used, section("kmod_tests"), aligned(8))) = { \
 		.name = #_name, \
-		.func = _name, \
+		.func = _func, \
 		## __VA_ARGS__ \
 	};
 
+#define DEFINE_TEST(_name, ...) DEFINE_TEST_WITH_FUNC(_name, _name, __VA_ARGS__)
+
 #define TESTSUITE_MAIN() \
 	extern struct test __start_kmod_tests[] __attribute__((weak, visibility("hidden")));	\
 	extern struct test __stop_kmod_tests[] __attribute__((weak, visibility("hidden")));	\
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] test-modprobe: share single function for kcmdline tests
  2021-02-12  9:45 [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
  2021-02-12  9:45 ` [PATCH 3/4] testsuite: allow to re-use single function for tests Lucas De Marchi
@ 2021-02-12  9:45 ` Lucas De Marchi
  2 siblings, 0 replies; 6+ messages in thread
From: Lucas De Marchi @ 2021-02-12  9:45 UTC (permalink / raw)
  To: linux-modules; +Cc: Jiri Slaby, Jessica Yu, Giovanni Gherdovich

---
 testsuite/test-modprobe.c | 95 +++++----------------------------------
 1 file changed, 10 insertions(+), 85 deletions(-)

diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index dbc54f3..0255f1a 100644
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -213,7 +213,7 @@ DEFINE_TEST(modprobe_install_cmd_loop,
 	.modules_loaded = "mod-loop-b,mod-loop-a",
 	);
 
-static noreturn int modprobe_param_kcmdline(const struct test *t)
+static noreturn int modprobe_param_kcmdline_show_deps(const struct test *t)
 {
 	const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
 	const char *const args[] = {
@@ -225,7 +225,7 @@ static noreturn int modprobe_param_kcmdline(const struct test *t)
 	test_spawn_prog(progname, args);
 	exit(EXIT_FAILURE);
 }
-DEFINE_TEST(modprobe_param_kcmdline,
+DEFINE_TEST(modprobe_param_kcmdline_show_deps,
 	.description = "check if params from kcmdline are passed to (f)init_module call",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -237,7 +237,7 @@ DEFINE_TEST(modprobe_param_kcmdline,
 	.modules_loaded = "",
 	);
 
-static noreturn int modprobe_param_kcmdline2(const struct test *t)
+static noreturn int modprobe_param_kcmdline(const struct test *t)
 {
 	const char *progname = ABS_TOP_BUILDDIR "/tools/modprobe";
 	const char *const args[] = {
@@ -249,7 +249,7 @@ static noreturn int modprobe_param_kcmdline2(const struct test *t)
 	test_spawn_prog(progname, args);
 	exit(EXIT_FAILURE);
 }
-DEFINE_TEST(modprobe_param_kcmdline2,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline2, modprobe_param_kcmdline,
 	.description = "check if params with no value are parsed correctly from kcmdline",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -261,19 +261,7 @@ DEFINE_TEST(modprobe_param_kcmdline2,
 	.modules_loaded = "",
 	);
 
-static noreturn int modprobe_param_kcmdline3(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_kcmdline3,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline3, modprobe_param_kcmdline,
 	.description = "check if unrelated strings in kcmdline are correctly ignored",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -285,19 +273,7 @@ DEFINE_TEST(modprobe_param_kcmdline3,
 	.modules_loaded = "",
 	);
 
-static noreturn int modprobe_param_kcmdline4(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_kcmdline4,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline4, modprobe_param_kcmdline,
 	.description = "check if unrelated strings in kcmdline are correctly ignored",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -309,19 +285,7 @@ DEFINE_TEST(modprobe_param_kcmdline4,
 	.modules_loaded = "",
 	);
 
-static noreturn int modprobe_param_kcmdline5(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_kcmdline5,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline5, modprobe_param_kcmdline,
 	.description = "check if params with spaces are parsed correctly from kcmdline",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -333,20 +297,7 @@ DEFINE_TEST(modprobe_param_kcmdline5,
 	.modules_loaded = "",
 	);
 
-
-static noreturn int modprobe_param_kcmdline6(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_kcmdline6,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline6, modprobe_param_kcmdline,
 	.description = "check if dots on other parts of kcmdline don't confuse our parser",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -358,20 +309,7 @@ DEFINE_TEST(modprobe_param_kcmdline6,
 	.modules_loaded = "",
 	);
 
-
-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,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline7, modprobe_param_kcmdline,
 	.description = "check if dots on other parts of kcmdline don't confuse our parser",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
@@ -383,20 +321,7 @@ DEFINE_TEST(modprobe_param_kcmdline7,
 	.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,
+DEFINE_TEST_WITH_FUNC(modprobe_param_kcmdline8, modprobe_param_kcmdline,
 	.description = "check if dots on other parts of kcmdline don't confuse our parser",
 	.config = {
 		[TC_UNAME_R] = "4.4.4",
-- 
2.30.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline
  2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
@ 2021-02-15 15:28   ` Jessica Yu
  2021-02-15 20:18     ` Lucas De Marchi
  0 siblings, 1 reply; 6+ messages in thread
From: Jessica Yu @ 2021-02-15 15:28 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules, Jiri Slaby, Giovanni Gherdovich

+++ Lucas De Marchi [12/02/21 01:45 -0800]:
>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>
>Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10

Hi Lucas,

Thanks a lot for working on this. I applied this patchset on top of
kmod master and after some light testing it appears to be able to
handle the mangled quoting from grub now:

     Tested-by: Jessica Yu <jeyu@kernel.org>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline
  2021-02-15 15:28   ` Jessica Yu
@ 2021-02-15 20:18     ` Lucas De Marchi
  0 siblings, 0 replies; 6+ messages in thread
From: Lucas De Marchi @ 2021-02-15 20:18 UTC (permalink / raw)
  To: Jessica Yu
  Cc: Lucas De Marchi, linux-modules, Jiri Slaby, Giovanni Gherdovich

On Mon, Feb 15, 2021 at 7:33 AM Jessica Yu <jeyu@kernel.org> wrote:
>
> +++ Lucas De Marchi [12/02/21 01:45 -0800]:
> >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>
> >Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10
>
> Hi Lucas,
>
> Thanks a lot for working on this. I applied this patchset on top of
> kmod master and after some light testing it appears to be able to
> handle the mangled quoting from grub now:
>
>      Tested-by: Jessica Yu <jeyu@kernel.org>


thanks!

Applied
Lucas De Marchi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-15 20:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12  9:45 [PATCH 1/4] libkmod-config: revamp kcmdline parsing into a state machine Lucas De Marchi
2021-02-12  9:45 ` [PATCH 2/4] libkmod-config: re-quote option from kernel cmdline Lucas De Marchi
2021-02-15 15:28   ` Jessica Yu
2021-02-15 20:18     ` Lucas De Marchi
2021-02-12  9:45 ` [PATCH 3/4] testsuite: allow to re-use single function for tests Lucas De Marchi
2021-02-12  9:45 ` [PATCH 4/4] test-modprobe: share single function for kcmdline tests Lucas De Marchi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.