[LTP] [PATCH 1/3] kconfig: add funtion to parse /proc/cmdline

[LTP] [PATCH 1/3] kconfig: add funtion to parse /proc/cmdline

[Li Wang]

In tst_kconfig.c, it adds the tst_kcmdline_parse function to
read and parse command-line parameters from /proc/cmdline.
This function tokenizes the command line, matches keys with
the provided parameter array, and stores the associated values.
The update enhances the testing framework's ability to handle
kernel configuration parameters dynamically.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 include/tst_kconfig.h | 23 +++++++++++++++++++++++
 lib/tst_kconfig.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
index 8b24a8380..0ecf2c0d1 100644
--- a/include/tst_kconfig.h
+++ b/include/tst_kconfig.h
@@ -64,4 +64,27 @@ void tst_kconfig_read(struct tst_kconfig_var vars[], size_t vars_len);
  */
 int tst_kconfig_check(const char *const kconfigs[]);
 
+/**
+ * Macro to initialize a tst_kcmdline_param structure with a specified parameter
+ * name and an empty value. This is useful for setting up an array of parameter
+ * structures before parsing the actual command-line arguments.
+ */
+#define TST_KCMDLINE_INIT(paraname) { \
+	.key = paraname, \
+	.value = "" \
+}
+
+/* Structure for storing command-line parameter key and its corresponding value */
+struct tst_kcmdline_param {
+	const char *key;
+	char value[128];
+};
+
+/**
+ * Parses command-line parameters from /proc/cmdline and stores them in params array
+ * params: The array of tst_kcmdline_param structures to be filled with parsed key-value pairs
+ * params_len: The length of the params array, indicating how many parameters to parse
+ */
+void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len);
+
 #endif	/* TST_KCONFIG_H__ */
diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 595ea4b09..f79dea5c6 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname)
 
 	return var.choice;
 }
+
+void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) {
+	FILE *proc_cmdline;
+	char cmdline[4096];
+	char *token, *key, *value;
+
+	proc_cmdline = fopen("/proc/cmdline", "r");
+	if (proc_cmdline == NULL)
+		tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading");
+
+	if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) {
+		fclose(proc_cmdline);
+
+		if (feof(proc_cmdline))
+			tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data");
+		else
+			tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline");
+	}
+	fclose(proc_cmdline);
+
+	token = strtok(cmdline, " ");
+	while (token != NULL) {
+		key = token;
+		value = strchr(token, '=');
+
+		if (value != NULL) {
+			/* Split the token into key and value at '=' */
+			*value++ = '\0';
+
+			for (size_t i = 0; i < params_len; i++) {
+				if (strcmp(params[i].key, key) == 0) {
+					strncpy(params[i].value, value, sizeof(params[i].value) - 1);
+					params[i].value[sizeof(params[i].value) - 1] = '\0';
+					break;
+				}
+			}
+		}
+
+		token = strtok(NULL, " ");
+	}
+}
-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] init_module: To handle kernel module signature enforcement

[Li Wang]

The patch modifies init_module syscall test cases to account
for kernel module signature enforcement. It adds parsing for
the 'module.sig_enforce' parameter and adjusts test expectations
based on whether signature enforcement is enabled, using
new conditional logic.

If enforcement is active, tests expect an EKEYREJECTED error;
otherwise, they proceed as normal.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 .../syscalls/delete_module/delete_module01.c  |  8 ++++
 .../syscalls/delete_module/delete_module03.c  |  8 ++++
 .../syscalls/finit_module/finit_module01.c    | 15 +++++++-
 .../syscalls/finit_module/finit_module02.c    | 37 ++++++++++++-------
 .../syscalls/init_module/init_module01.c      | 13 +++++++
 .../syscalls/init_module/init_module02.c      | 19 +++++++++-
 6 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
index 90d8b5289..4c31f81b0 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module01.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
@@ -14,8 +14,10 @@
  * Install dummy_del_mod.ko and delete it with delete_module(2).
  */
 
+#include <stdlib.h>
 #include "tst_test.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "lapi/syscalls.h"
 
 #define MODULE_NAME	"dummy_del_mod"
@@ -25,6 +27,12 @@ static int module_loaded;
 
 static void do_delete_module(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	if (atoi(params.value) == 1)
+		tst_brk(TCONF, "module signature is enforced, skip test");
+
 	if (!module_loaded) {
 		tst_module_load(MODULE_NAME_KO, NULL);
 		module_loaded = 1;
diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
index 7e92fc2af..66a1fb815 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module03.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
@@ -12,9 +12,11 @@
  * if tried to remove a module while other modules depend on this module.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "tst_test.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "lapi/syscalls.h"
 
 #define DUMMY_MOD		"dummy_del_mod"
@@ -50,6 +52,12 @@ static void do_delete_module(void)
 
 static void setup(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	if (atoi(params.value) == 1)
+		tst_brk(TCONF, "module signature is enforced, skip test");
+
 	/* Load first kernel module */
 	tst_module_load(DUMMY_MOD_KO, NULL);
 	dummy_mod_loaded = 1;
diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
index 1929c30fa..88d84297f 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module01.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
@@ -13,18 +13,25 @@
  * Inserts a simple module after opening and mmaping the module file.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 
 #define MODULE_NAME	"finit_module.ko"
 
-static int fd;
+static int fd, sig_enforce;
 
 static char *mod_path;
 
 static void setup(void)
 {
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
+
 	tst_module_exists(MODULE_NAME, &mod_path);
 
 	fd = SAFE_OPEN(mod_path, O_RDONLY|O_CLOEXEC);
@@ -32,6 +39,12 @@ static void setup(void)
 
 static void run(void)
 {
+	if (sig_enforce == 1) {
+		tst_res(TINFO, "module signature is enforced");
+		TST_EXP_FAIL(finit_module(fd, "status=valid", 0), EKEYREJECTED);
+		return;
+	}
+
 	TST_EXP_PASS(finit_module(fd, "status=valid", 0));
 	if (!TST_PASS)
 		return;
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index 223d9b388..0e9a787f0 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -14,9 +14,11 @@
  */
 
 #include <linux/capability.h>
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 #include "tst_capability.h"
 
 #define MODULE_NAME	"finit_module.ko"
@@ -25,7 +27,7 @@
 static char *mod_path;
 
 static int fd, fd_zero, fd_invalid = -1, fd_dir;
-static int kernel_lockdown, secure_boot;
+static int kernel_lockdown, secure_boot, sig_enforce;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
 static struct tst_cap cap_drop = TST_CAP(TST_CAP_DROP, CAP_SYS_MODULE);
@@ -59,27 +61,26 @@ static void dir_setup(struct tcase *tc)
 }
 
 static struct tcase tcases[] = {
-	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0,
-		bad_fd_setup},
+	{"invalid-fd", &fd_invalid, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0, bad_fd_setup},
 	{"zero-fd", &fd_zero, "", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, 0, NULL},
 	{"null-param", &fd, NULL, O_RDONLY | O_CLOEXEC, 0, 0, EFAULT, 1, NULL},
-	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0,
-		EINVAL, 1, NULL},
-	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, 0,
-		NULL},
+	{"invalid-param", &fd, "status=invalid", O_RDONLY | O_CLOEXEC, 0, 0, EINVAL, 1, NULL},
+	{"invalid-flags", &fd, "", O_RDONLY | O_CLOEXEC, -1, 0, EINVAL, 0, NULL},
 	{"no-perm", &fd, "", O_RDONLY | O_CLOEXEC, 0, 1, EPERM, 0, NULL},
-	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, 1,
-		NULL},
-	{"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF, 0,
-		NULL},
-	{"file-readwrite", &fd, "", O_RDWR | O_CLOEXEC, 0, 0, ETXTBSY, 0,
-		NULL},
+	{"module-exists", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EEXIST, 1, NULL},
+	{"module-unsigned", &fd, "", O_RDONLY | O_CLOEXEC, 0, 0, EKEYREJECTED, 1, NULL},
+	{"file-not-readable", &fd, "", O_WRONLY | O_CLOEXEC, 0, 0, EBADF, 0, NULL},
+	{"file-readwrite", &fd, "", O_RDWR | O_CLOEXEC, 0, 0, ETXTBSY, 0, NULL},
 	{"directory", &fd_dir, "", O_RDONLY | O_CLOEXEC, 0, 0, 0, 0, dir_setup},
 };
 
 static void setup(void)
 {
 	unsigned long int i;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, &mod_path);
 
@@ -109,6 +110,16 @@ static void run(unsigned int n)
 		return;
 	}
 
+	if ((sig_enforce == 1) && (tc->exp_errno != EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is enforced, skipping %s", tc->name);
+		return;
+	}
+
+	if ((sig_enforce != 1) && (tc->exp_errno == EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is not enforced, skipping %s", tc->name);
+		return;
+	}
+
 	fd = SAFE_OPEN(mod_path, tc->open_flags);
 
 	if (tc->cap)
diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
index 26ff0b93b..deba47571 100644
--- a/testcases/kernel/syscalls/init_module/init_module01.c
+++ b/testcases/kernel/syscalls/init_module/init_module01.c
@@ -13,18 +13,25 @@
  * Inserts a simple module after opening and mmaping the module file.
  */
 
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
+#include "tst_kconfig.h"
 
 #define MODULE_NAME	"init_module.ko"
 
 static struct stat sb;
 static void *buf;
+static int sig_enforce;
 
 static void setup(void)
 {
 	int fd;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, NULL);
 
@@ -36,6 +43,12 @@ static void setup(void)
 
 static void run(void)
 {
+	if (sig_enforce == 1) {
+		tst_res(TINFO, "module signature is enforced");
+		TST_EXP_FAIL(init_module(buf, sb.st_size, "status=valid"), EKEYREJECTED);
+		return;
+	}
+
 	TST_EXP_PASS(init_module(buf, sb.st_size, "status=valid"));
 	if (!TST_PASS)
 		return;
diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
index e6730e21c..a913c0b4c 100644
--- a/testcases/kernel/syscalls/init_module/init_module02.c
+++ b/testcases/kernel/syscalls/init_module/init_module02.c
@@ -14,15 +14,17 @@
  */
 
 #include <linux/capability.h>
+#include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
+#include "tst_kconfig.h"
 #include "tst_module.h"
 #include "tst_capability.h"
 
 #define MODULE_NAME	"init_module.ko"
 
 static unsigned long size, zero_size;
-static int kernel_lockdown, secure_boot;
+static int kernel_lockdown, secure_boot, sig_enforce;
 static void *buf, *faulty_buf, *null_buf;
 
 static struct tst_cap cap_req = TST_CAP(TST_CAP_REQ, CAP_SYS_MODULE);
@@ -44,12 +46,17 @@ static struct tcase {
 	{"invalid_param", &buf, &size, "status=invalid", 0, 1, EINVAL},
 	{"no-perm", &buf, &size, "", 1, 0, EPERM},
 	{"module-exists", &buf, &size, "", 0, 1, EEXIST},
+	{"module-unsigned", &buf, &size, "", 0, 1, EKEYREJECTED},
 };
 
 static void setup(void)
 {
 	struct stat sb;
 	int fd;
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
+
+	tst_kcmdline_parse(&params, 1);
+	sig_enforce = atoi(params.value);
 
 	tst_module_exists(MODULE_NAME, NULL);
 
@@ -73,6 +80,16 @@ static void run(unsigned int n)
 		return;
 	}
 
+	if ((sig_enforce == 1) && (tc->exp_errno != EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is enforced, skipping %s", tc->name);
+		return;
+	}
+
+	if ((sig_enforce != 1) && (tc->exp_errno == EKEYREJECTED)) {
+		tst_res(TCONF, "module signature is not enforced, skipping %s", tc->name);
+		return;
+	}
+
 	if (tc->cap)
 		tst_cap_action(&cap_drop);
 
-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] stack_clash: make use of tst_kcmdline_parse

[Li Wang]

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/cve/stack_clash.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/testcases/cve/stack_clash.c b/testcases/cve/stack_clash.c
index 56b970a1b..ae70dd866 100644
--- a/testcases/cve/stack_clash.c
+++ b/testcases/cve/stack_clash.c
@@ -44,6 +44,7 @@
 #include <stdlib.h>
 
 #include "tst_test.h"
+#include "tst_kconfig.h"
 #include "tst_safe_stdio.h"
 #include "lapi/mmap.h"
 
@@ -272,19 +273,16 @@ void do_child(void)
 
 void setup(void)
 {
-	char buf[4096], *p;
+	char *p;
 
 	page_size = sysconf(_SC_PAGESIZE);
 	page_mask = ~(page_size - 1);
 
-	buf[4095] = '\0';
-	SAFE_FILE_SCANF("/proc/cmdline", "%4095[^\n]", buf);
+	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("stack_guard_gap");
+	tst_kcmdline_parse(&params, 1);
 
-	if ((p = strstr(buf, "stack_guard_gap=")) != NULL) {
-		if (sscanf(p, "stack_guard_gap=%ld", &GAP_PAGES) != 1) {
-			tst_brk(TBROK | TERRNO, "sscanf");
-			return;
-		}
+	if (params.value[0] != '\0') {
+		GAP_PAGES= atol(params.value);
 		tst_res(TINFO, "stack_guard_gap = %ld", GAP_PAGES);
 	}
 
-- 
2.40.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] kconfig: add funtion to parse /proc/cmdline

[Petr Vorel]

Hi Li,

> In tst_kconfig.c, it adds the tst_kcmdline_parse function to
> read and parse command-line parameters from /proc/cmdline.
> This function tokenizes the command line, matches keys with
> the provided parameter array, and stores the associated values.
> The update enhances the testing framework's ability to handle
> kernel configuration parameters dynamically.

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  include/tst_kconfig.h | 23 +++++++++++++++++++++++
>  lib/tst_kconfig.c     | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+)

> diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
> index 8b24a8380..0ecf2c0d1 100644
> --- a/include/tst_kconfig.h
> +++ b/include/tst_kconfig.h
> @@ -64,4 +64,27 @@ void tst_kconfig_read(struct tst_kconfig_var vars[], size_t vars_len);
>   */
>  int tst_kconfig_check(const char *const kconfigs[]);

> +/**
> + * Macro to initialize a tst_kcmdline_param structure with a specified parameter
> + * name and an empty value. This is useful for setting up an array of parameter
> + * structures before parsing the actual command-line arguments.
> + */
> +#define TST_KCMDLINE_INIT(paraname) { \
> +	.key = paraname, \
> +	.value = "" \
> +}
> +
> +/* Structure for storing command-line parameter key and its corresponding value */
> +struct tst_kcmdline_param {
> +	const char *key;
> +	char value[128];
> +};
> +
> +/**
> + * Parses command-line parameters from /proc/cmdline and stores them in params array
> + * params: The array of tst_kcmdline_param structures to be filled with parsed key-value pairs
> + * params_len: The length of the params array, indicating how many parameters to parse
> + */
> +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len);
> +
>  #endif	/* TST_KCONFIG_H__ */
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 595ea4b09..f79dea5c6 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname)

>  	return var.choice;
>  }
> +
> +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) {
> +	FILE *proc_cmdline;
> +	char cmdline[4096];
> +	char *token, *key, *value;
> +
> +	proc_cmdline = fopen("/proc/cmdline", "r");
Why not SAFE_FOPEN() ?
> +	if (proc_cmdline == NULL)
> +		tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading");
> +
> +	if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) {
> +		fclose(proc_cmdline);
Maybe SAFE_FCLOSE() ?
> +
> +		if (feof(proc_cmdline))
tst_kconfig.c:581:21: warning: pointer ‘proc_cmdline’ used after ‘fclose’ [-Wuse-after-free]
  581 |                 if (feof(proc_cmdline))
      |                     ^~~~~~~~~~~~~~~~~~
tst_kconfig.c:579:17: note: call to ‘fclose’ here
  579 |                 fclose(proc_cmdline);
      |                 ^~~~~~~~~~~~~~~~~~~~

> +			tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data");
> +		else
> +			tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline");
> +	}
> +	fclose(proc_cmdline);
Maybe SAFE_FCLOSE() ?

Kind regards,
Petr

> +
> +	token = strtok(cmdline, " ");
> +	while (token != NULL) {
> +		key = token;
> +		value = strchr(token, '=');
> +
> +		if (value != NULL) {
> +			/* Split the token into key and value at '=' */
> +			*value++ = '\0';
> +
> +			for (size_t i = 0; i < params_len; i++) {
> +				if (strcmp(params[i].key, key) == 0) {
> +					strncpy(params[i].value, value, sizeof(params[i].value) - 1);
> +					params[i].value[sizeof(params[i].value) - 1] = '\0';
> +					break;
> +				}
> +			}
> +		}
> +
> +		token = strtok(NULL, " ");
> +	}
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] init_module: To handle kernel module signature enforcement

[Petr Vorel]

Hi Li,

> The patch modifies init_module syscall test cases to account
> for kernel module signature enforcement. It adds parsing for
> the 'module.sig_enforce' parameter and adjusts test expectations
> based on whether signature enforcement is enabled, using
> new conditional logic.

> If enforcement is active, tests expect an EKEYREJECTED error;
> otherwise, they proceed as normal.

...
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
> index 90d8b5289..4c31f81b0 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> @@ -14,8 +14,10 @@
>   * Install dummy_del_mod.ko and delete it with delete_module(2).
>   */

> +#include <stdlib.h>
>  #include "tst_test.h"
>  #include "tst_module.h"
> +#include "tst_kconfig.h"
>  #include "lapi/syscalls.h"

>  #define MODULE_NAME	"dummy_del_mod"
> @@ -25,6 +27,12 @@ static int module_loaded;

>  static void do_delete_module(void)
>  {
> +	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("module.sig_enforce");
> +
> +	tst_kcmdline_parse(&params, 1);
> +	if (atoi(params.value) == 1)
> +		tst_brk(TCONF, "module signature is enforced, skip test");
Only 2 tests do tst_brk(TCONF). I was thinking about adding library flag
.skip_in_module_sig_enforce, but probably not worth of it.

The rest LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/3] stack_clash: make use of tst_kcmdline_parse

[Petr Vorel]

Hi Li,

>  void setup(void)
>  {
> -	char buf[4096], *p;
> +	char *p;
stack_clash.c:276:15: warning: unused variable ‘p’ [-Wunused-variable]
  276 |         char *p;
      |               ^

>  	page_size = sysconf(_SC_PAGESIZE);
>  	page_mask = ~(page_size - 1);

> -	buf[4095] = '\0';
> -	SAFE_FILE_SCANF("/proc/cmdline", "%4095[^\n]", buf);
> +	struct tst_kcmdline_param params = TST_KCMDLINE_INIT("stack_guard_gap");
> +	tst_kcmdline_parse(&params, 1);

> -	if ((p = strstr(buf, "stack_guard_gap=")) != NULL) {
> -		if (sscanf(p, "stack_guard_gap=%ld", &GAP_PAGES) != 1) {
> -			tst_brk(TBROK | TERRNO, "sscanf");
> -			return;
> -		}
> +	if (params.value[0] != '\0') {
> +		GAP_PAGES= atol(params.value);
>  		tst_res(TINFO, "stack_guard_gap = %ld", GAP_PAGES);
>  	}

Otherwise LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] kconfig: add funtion to parse /proc/cmdline

[Cyril Hrubis]

Hi!
> +/**
> + * Macro to initialize a tst_kcmdline_param structure with a specified parameter
> + * name and an empty value. This is useful for setting up an array of parameter
> + * structures before parsing the actual command-line arguments.
> + */
> +#define TST_KCMDLINE_INIT(paraname) { \
> +	.key = paraname, \
> +	.value = "" \
> +}
> +
> +/* Structure for storing command-line parameter key and its corresponding value */
> +struct tst_kcmdline_param {
                       ^
		       maybe var as short for variable would be better
		       name
> +	const char *key;
> +	char value[128];
> +};
> +
> +/**
> + * Parses command-line parameters from /proc/cmdline and stores them in params array
> + * params: The array of tst_kcmdline_param structures to be filled with parsed key-value pairs
> + * params_len: The length of the params array, indicating how many parameters to parse
> + */
> +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len);
> +
>  #endif	/* TST_KCONFIG_H__ */
> diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> index 595ea4b09..f79dea5c6 100644
> --- a/lib/tst_kconfig.c
> +++ b/lib/tst_kconfig.c
> @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname)
>  
>  	return var.choice;
>  }
> +
> +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t params_len) {
> +	FILE *proc_cmdline;
> +	char cmdline[4096];
> +	char *token, *key, *value;
> +
> +	proc_cmdline = fopen("/proc/cmdline", "r");
> +	if (proc_cmdline == NULL)
> +		tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for reading");
> +
> +	if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) {
> +		fclose(proc_cmdline);
> +
> +		if (feof(proc_cmdline))
> +			tst_brk(TBROK, "End-of-File reached on /proc/cmdline without reading any data");
> +		else
> +			tst_brk(TBROK | TERRNO, "Failed to read from /proc/cmdline");
> +	}
> +	fclose(proc_cmdline);

Uff, this is ugly. We have FILE and then use it as if we had fd and read
it as a whole. The whole point of FILE is that glibc manages the buffers
and reads so that we can access it character by character without being
slow. It would be way cleaner if we just read the file character by
character building up the key and value while we do that.

Something as (bevare untested):

	char buf[128];
	size_t buf_pos = 0, i;
	int var_id = -1;

	f = fopen("/proc/cmdline", "r");

	while ((c = fgetc(f)) != EOF) {
		switch (c) {
		case '=':
			buf[buf_pos] = 0;

			for (i = 0; i < vars_len; i++)
				var_id = i;
		break;
		case ' ':
			buf[buf_pos] = 0;

			if (var_id >= 0)
				strcpy(vars[var_id].val, buf);

			var_id = -1;
		break;
		default:
			if (buf_pos+1 >= sizeof(buf))
				//warning?

			buf[buf_pos++] = c;
		break;
		}
	}


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/3] kconfig: add funtion to parse /proc/cmdline

[Li Wang]

On Sat, Mar 9, 2024 at 4:46 AM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > +/**
> > + * Macro to initialize a tst_kcmdline_param structure with a specified
> parameter
> > + * name and an empty value. This is useful for setting up an array of
> parameter
> > + * structures before parsing the actual command-line arguments.
> > + */
> > +#define TST_KCMDLINE_INIT(paraname) { \
> > +     .key = paraname, \
> > +     .value = "" \
> > +}
> > +
> > +/* Structure for storing command-line parameter key and its
> corresponding value */
> > +struct tst_kcmdline_param {
>                        ^
>                        maybe var as short for variable would be better
>                        name
> > +     const char *key;
> > +     char value[128];
> > +};
> > +
> > +/**
> > + * Parses command-line parameters from /proc/cmdline and stores them in
> params array
> > + * params: The array of tst_kcmdline_param structures to be filled with
> parsed key-value pairs
> > + * params_len: The length of the params array, indicating how many
> parameters to parse
> > + */
> > +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t
> params_len);
> > +
> >  #endif       /* TST_KCONFIG_H__ */
> > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> > index 595ea4b09..f79dea5c6 100644
> > --- a/lib/tst_kconfig.c
> > +++ b/lib/tst_kconfig.c
> > @@ -565,3 +565,44 @@ char tst_kconfig_get(const char *confname)
> >
> >       return var.choice;
> >  }
> > +
> > +void tst_kcmdline_parse(struct tst_kcmdline_param params[], size_t
> params_len) {
> > +     FILE *proc_cmdline;
> > +     char cmdline[4096];
> > +     char *token, *key, *value;
> > +
> > +     proc_cmdline = fopen("/proc/cmdline", "r");
> > +     if (proc_cmdline == NULL)
> > +             tst_brk(TBROK | TERRNO, "Failed to open /proc/cmdline for
> reading");
> > +
> > +     if (fgets(cmdline, sizeof(cmdline), proc_cmdline) == NULL) {
> > +             fclose(proc_cmdline);
> > +
> > +             if (feof(proc_cmdline))
> > +                     tst_brk(TBROK, "End-of-File reached on
> /proc/cmdline without reading any data");
> > +             else
> > +                     tst_brk(TBROK | TERRNO, "Failed to read from
> /proc/cmdline");
> > +     }
> > +     fclose(proc_cmdline);
>
> Uff, this is ugly. We have FILE and then use it as if we had fd and read
> it as a whole. The whole point of FILE is that glibc manages the buffers
> and reads so that we can access it character by character without being
> slow. It would be way cleaner if we just read the file character by
> character building up the key and value while we do that.
>

Indeed, this is much better than my way. Thanks!


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] init_module: To handle kernel module signature enforcement

[Li Wang]

On Fri, Mar 8, 2024 at 4:47 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > The patch modifies init_module syscall test cases to account
> > for kernel module signature enforcement. It adds parsing for
> > the 'module.sig_enforce' parameter and adjusts test expectations
> > based on whether signature enforcement is enabled, using
> > new conditional logic.
>
> > If enforcement is active, tests expect an EKEYREJECTED error;
> > otherwise, they proceed as normal.
>
> ...
> > diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c
> b/testcases/kernel/syscalls/delete_module/delete_module01.c
> > index 90d8b5289..4c31f81b0 100644
> > --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> > +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> > @@ -14,8 +14,10 @@
> >   * Install dummy_del_mod.ko and delete it with delete_module(2).
> >   */
>
> > +#include <stdlib.h>
> >  #include "tst_test.h"
> >  #include "tst_module.h"
> > +#include "tst_kconfig.h"
> >  #include "lapi/syscalls.h"
>
> >  #define MODULE_NAME  "dummy_del_mod"
> > @@ -25,6 +27,12 @@ static int module_loaded;
>
> >  static void do_delete_module(void)
> >  {
> > +     struct tst_kcmdline_param params =
> TST_KCMDLINE_INIT("module.sig_enforce");
> > +
> > +     tst_kcmdline_parse(&params, 1);
> > +     if (atoi(params.value) == 1)
> > +             tst_brk(TCONF, "module signature is enforced, skip test");
> Only 2 tests do tst_brk(TCONF). I was thinking about adding library flag
> .skip_in_module_sig_enforce, but probably not worth of it.
>

Hmm, it is not very necessary at this moment.
Maybe in the future, it could be an extended direction.



>
> The rest LGTM.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
>

Thanks a lot for testing/reviewing, the reset comments look good to me.

V2 is coming.


-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp