[LTP] [PATCH 0/2] module signature enforcement helpers

public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed

* [LTP] [PATCH 0/2] module signature enforcement helpers
@ 2024-12-27 16:00 Petr Vorel
  2024-12-27 16:00 ` [LTP] [PATCH 1/2] lib: Add helpers for module signature enforcement Petr Vorel
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Petr Vorel @ 2024-12-27 16:00 UTC (permalink / raw)
  To: ltp; +Cc: Po-Hsu Lin, Ricardo B . Marliere

Petr Vorel (2):
  lib: Add helpers for module signature enforcement
  device-drivers: *module: Use helpers for module signature enforcement

 include/old/old_module.h                      | 33 +++++++++++++++++++
 include/tst_module.h                          | 17 ++++++++++
 lib/tst_module.c                              | 24 ++++++++++++++
 .../kernel/device-drivers/acpi/ltp_acpi.c     | 11 +------
 .../block/block_dev_user/block_dev.c          |  9 +----
 .../device-drivers/pci/tpci_user/tpci.c       | 11 +------
 .../kernel/device-drivers/uaccess/uaccess.c   | 11 +------
 .../kernel/firmware/fw_load_user/fw_load.c    | 10 +-----
 .../syscalls/delete_module/delete_module01.c  |  9 +----
 .../syscalls/delete_module/delete_module03.c  |  9 +----
 .../syscalls/finit_module/finit_module01.c    | 11 +------
 .../syscalls/finit_module/finit_module02.c    |  7 +---
 .../syscalls/init_module/init_module01.c      |  8 +----
 .../syscalls/init_module/init_module02.c      |  7 +---
 14 files changed, 85 insertions(+), 92 deletions(-)

-- 
2.45.2


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

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

* [LTP] [PATCH 1/2] lib: Add helpers for module signature enforcement
  2024-12-27 16:00 [LTP] [PATCH 0/2] module signature enforcement helpers Petr Vorel
@ 2024-12-27 16:00 ` Petr Vorel
  2024-12-27 16:00 ` [LTP] [PATCH 2/2] device-drivers: *module: Use " Petr Vorel
  2024-12-31  4:34 ` Po-Hsu Lin
  2 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2024-12-27 16:00 UTC (permalink / raw)
  To: ltp; +Cc: Po-Hsu Lin, Ricardo B . Marliere

Add them to both legacy and new API (used in both).

NOTE: because test is called often in the setup function
don't call a cleanup function in the old API version.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 include/old/old_module.h | 33 +++++++++++++++++++++++++++++++++
 include/tst_module.h     | 17 +++++++++++++++++
 lib/tst_module.c         | 24 ++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/include/old/old_module.h b/include/old/old_module.h
index 496520d64d..b2ea06d0c2 100644
--- a/include/old/old_module.h
+++ b/include/old/old_module.h
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
+ * Copyright (c) Linux Test Project, 2016-2024
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -34,6 +35,8 @@
 #ifndef TST_MODULE
 #define TST_MODULE
 
+#include <stdbool.h>
+
 void tst_module_exists_(void (cleanup_fn)(void), const char *mod_name,
 					 char **mod_path);
 
@@ -42,6 +45,9 @@ void tst_module_load_(void (cleanup_fn)(void), const char *mod_name,
 
 void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name);
 
+bool tst_module_signature_enforced_(void);
+void tst_check_module_signature_enforced_(void);
+
 /*
  * Check module existence.
  *
@@ -86,4 +92,31 @@ static inline void tst_module_unload(void (cleanup_fn)(void), const char *mod_na
 	tst_module_unload_(cleanup_fn, mod_name);
 }
 
+/**
+ * tst_check_module_signature_enforced() - Check if enforced module signature.
+ *
+ * Module signature is enforced if module.sig_enforce=1 kernel parameter or
+ * CONFIG_MODULE_SIG_FORCE=y.
+ *
+ * return: Returns true if module signature is enforced false otherwise.
+ *
+ */
+static inline bool tst_module_signature_enforced(void)
+{
+	return tst_module_signature_enforced_();
+}
+
+/**
+ * tst_check_module_signature_enforced() - Check if test needs to be skipped due
+ * enforced module signature.
+ *
+ * Skip test with tst_brk(TCONF) due module signature enforcement if
+ * module.sig_enforce=1 kernel parameter or CONFIG_MODULE_SIG_FORCE=y.
+ */
+
+static inline void tst_check_module_signature_enforced(void)
+{
+	tst_check_module_signature_enforced_();
+}
+
 #endif /* TST_MODULE */
diff --git a/include/tst_module.h b/include/tst_module.h
index 2654c5afb6..8353742e1c 100644
--- a/include/tst_module.h
+++ b/include/tst_module.h
@@ -1,12 +1,15 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
+ * Copyright (c) Linux Test Project, 2016-2024
  * Alexey Kodanev <alexey.kodanev@oracle.com>
  */
 
 #ifndef TST_MODULE_H
 #define TST_MODULE_H
 
+#include <stdbool.h>
+
 void tst_module_exists_(void (cleanup_fn)(void), const char *mod_name,
 					 char **mod_path);
 
@@ -30,4 +33,18 @@ static inline void tst_module_unload(const char *mod_name)
 	tst_module_unload_(NULL, mod_name);
 }
 
+bool tst_module_signature_enforced_(void);
+
+static inline bool tst_module_signature_enforced(void)
+{
+	return tst_module_signature_enforced_();
+}
+
+void tst_check_module_signature_enforced_(void);
+
+static inline void tst_check_module_signature_enforced(void)
+{
+	tst_check_module_signature_enforced_();
+}
+
 #endif /* TST_MODULE_H */
diff --git a/lib/tst_module.c b/lib/tst_module.c
index 9bd4436236..caf311f803 100644
--- a/lib/tst_module.c
+++ b/lib/tst_module.c
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved.
+ * Copyright (c) Linux Test Project, 2016-2024
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -23,8 +24,10 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
+#include <stdbool.h>
 
 #include "test.h"
+#include "tst_kconfig.h"
 #include "ltp_priv.h"
 #include "old_module.h"
 
@@ -122,3 +125,24 @@ void tst_module_unload_(void (cleanup_fn)(void), const char *mod_name)
 			 "could not unload %s module", mod_name);
 	}
 }
+
+bool tst_module_signature_enforced_(void)
+{
+	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
+	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
+	int rc;
+
+	tst_kcmdline_parse(&params, 1);
+	tst_kconfig_read(&kconfig, 1);
+
+	rc = params.found || kconfig.choice == 'y';
+	tst_resm(TINFO, "module signature enforcement: %s", rc ? "on" : "off");
+
+	return rc;
+}
+
+void tst_check_module_signature_enforced_(void)
+{
+	if (tst_module_signature_enforced_())
+		tst_brkm(TCONF, NULL, "module signature is enforced, skip test");
+}
-- 
2.45.2


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

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

* [LTP] [PATCH 2/2] device-drivers: *module: Use helpers for module signature enforcement
  2024-12-27 16:00 [LTP] [PATCH 0/2] module signature enforcement helpers Petr Vorel
  2024-12-27 16:00 ` [LTP] [PATCH 1/2] lib: Add helpers for module signature enforcement Petr Vorel
@ 2024-12-27 16:00 ` Petr Vorel
  2024-12-31  4:34 ` Po-Hsu Lin
  2 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2024-12-27 16:00 UTC (permalink / raw)
  To: ltp; +Cc: Po-Hsu Lin, Ricardo B . Marliere

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/device-drivers/acpi/ltp_acpi.c       | 11 +----------
 .../device-drivers/block/block_dev_user/block_dev.c   |  9 +--------
 testcases/kernel/device-drivers/pci/tpci_user/tpci.c  | 11 +----------
 testcases/kernel/device-drivers/uaccess/uaccess.c     | 11 +----------
 testcases/kernel/firmware/fw_load_user/fw_load.c      | 10 +---------
 .../kernel/syscalls/delete_module/delete_module01.c   |  9 +--------
 .../kernel/syscalls/delete_module/delete_module03.c   |  9 +--------
 .../kernel/syscalls/finit_module/finit_module01.c     | 11 +----------
 .../kernel/syscalls/finit_module/finit_module02.c     |  7 +------
 testcases/kernel/syscalls/init_module/init_module01.c |  8 +-------
 testcases/kernel/syscalls/init_module/init_module02.c |  7 +------
 11 files changed, 11 insertions(+), 92 deletions(-)

diff --git a/testcases/kernel/device-drivers/acpi/ltp_acpi.c b/testcases/kernel/device-drivers/acpi/ltp_acpi.c
index c674b20321..eb5e197239 100644
--- a/testcases/kernel/device-drivers/acpi/ltp_acpi.c
+++ b/testcases/kernel/device-drivers/acpi/ltp_acpi.c
@@ -22,7 +22,6 @@
 #include <stdlib.h>
 
 #include "test.h"
-#include "tst_kconfig.h"
 #include "old_module.h"
 #include "safe_macros.h"
 
@@ -129,20 +128,12 @@ static void test_run(void)
 int main(int argc, char *argv[])
 {
 	int acpi_disabled;
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
 
 	tst_parse_opts(argc, argv, NULL, NULL);
-
 	tst_require_root();
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
-
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
+	tst_check_module_signature_enforced();
 	tst_module_load(NULL, module_name, NULL);
 	module_loaded = 1;
 
diff --git a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
index 2c3ffd9203..98b786a90b 100644
--- a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
+++ b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
@@ -17,7 +17,6 @@
 #include <unistd.h>
 #include <string.h>
 
-#include "tst_kconfig.h"
 #include "tst_test.h"
 #include "tst_module.h"
 
@@ -42,13 +41,7 @@ static void cleanup(void)
 
 static void run(unsigned int n)
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brk(TCONF, "module signature is enforced, skip test");
+	tst_check_module_signature_enforced();
 
 	/*
 	 * test-cases #8 and #9 can crash the kernel.
diff --git a/testcases/kernel/device-drivers/pci/tpci_user/tpci.c b/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
index aa07fdb427..1bcc68c23c 100644
--- a/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
+++ b/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
@@ -27,7 +27,6 @@
 #include <errno.h>
 
 #include "test.h"
-#include "tst_kconfig.h"
 #include "safe_macros.h"
 #include "old_module.h"
 
@@ -50,16 +49,8 @@ static void cleanup(void)
 
 void setup(void)
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
 	tst_require_root();
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
-
+	tst_check_module_signature_enforced();
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 }
 
diff --git a/testcases/kernel/device-drivers/uaccess/uaccess.c b/testcases/kernel/device-drivers/uaccess/uaccess.c
index c6dfeb8456..58bef13ef3 100644
--- a/testcases/kernel/device-drivers/uaccess/uaccess.c
+++ b/testcases/kernel/device-drivers/uaccess/uaccess.c
@@ -27,7 +27,6 @@
 #include <unistd.h>
 
 #include "test.h"
-#include "tst_kconfig.h"
 #include "old_module.h"
 #include "safe_macros.h"
 
@@ -93,20 +92,12 @@ static void tc_write_userspace(void)
 
 int main(int argc, char *argv[])
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
 	tst_parse_opts(argc, argv, NULL, NULL);
 
 	tst_require_root();
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
-
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
+	tst_check_module_signature_enforced();
 	tst_module_load(NULL, module_name, NULL);
 	module_loaded = 1;
 
diff --git a/testcases/kernel/firmware/fw_load_user/fw_load.c b/testcases/kernel/firmware/fw_load_user/fw_load.c
index b34b56fae5..7ebb2cc8ed 100644
--- a/testcases/kernel/firmware/fw_load_user/fw_load.c
+++ b/testcases/kernel/firmware/fw_load_user/fw_load.c
@@ -29,7 +29,6 @@
 #include <string.h>
 
 #include "test.h"
-#include "tst_kconfig.h"
 #include "safe_macros.h"
 #include "old_module.h"
 
@@ -103,9 +102,6 @@ static void help(void)
 
 void setup(int argc, char *argv[])
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
 	tst_parse_opts(argc, argv, options, help);
 
 	if (nflag) {
@@ -116,11 +112,7 @@ void setup(int argc, char *argv[])
 	}
 
 	tst_require_root();
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
+	tst_check_module_signature_enforced();
 
 	char fw_size_param[19];
 	snprintf(fw_size_param, 19, "fw_size=%d", fw_size);
diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
index 5ccbe6fd5b..48b324309f 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module01.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
@@ -17,7 +17,6 @@
 #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"
@@ -27,13 +26,7 @@ static int module_loaded;
 
 static void do_delete_module(void)
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brk(TCONF, "module signature is enforced, skip test");
+	tst_check_module_signature_enforced();
 
 	if (!module_loaded) {
 		tst_module_load(MODULE_NAME_KO, NULL);
diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
index 76ccfb1e25..be358381ef 100644
--- a/testcases/kernel/syscalls/delete_module/delete_module03.c
+++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
@@ -16,7 +16,6 @@
 #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"
@@ -52,13 +51,7 @@ static void do_delete_module(void)
 
 static void setup(void)
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		tst_brk(TCONF, "module signature is enforced, skip test");
+	tst_check_module_signature_enforced();
 
 	/* Load first kernel module */
 	tst_module_load(DUMMY_MOD_KO, NULL);
diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
index d75e5e4753..0ed68c7372 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module01.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
@@ -17,7 +17,6 @@
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
-#include "tst_kconfig.h"
 
 #define MODULE_NAME	"finit_module.ko"
 
@@ -27,14 +26,7 @@ static char *mod_path;
 
 static void setup(void)
 {
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
-
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
-		sig_enforce = 1;
-
+	tst_check_module_signature_enforced();
 	tst_module_exists(MODULE_NAME, &mod_path);
 
 	fd = SAFE_OPEN(mod_path, O_RDONLY|O_CLOEXEC);
@@ -43,7 +35,6 @@ 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;
 	}
diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
index dc81ae727d..e16164ee25 100644
--- a/testcases/kernel/syscalls/finit_module/finit_module02.c
+++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
@@ -18,7 +18,6 @@
 #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"
@@ -77,12 +76,8 @@ static struct tcase tcases[] = {
 static void setup(void)
 {
 	unsigned long int i;
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
 
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
+	if (tst_module_signature_enforced())
 		sig_enforce = 1;
 
 	tst_module_exists(MODULE_NAME, &mod_path);
diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
index d7ed08b143..850f826e62 100644
--- a/testcases/kernel/syscalls/init_module/init_module01.c
+++ b/testcases/kernel/syscalls/init_module/init_module01.c
@@ -17,7 +17,6 @@
 #include <errno.h>
 #include "lapi/init_module.h"
 #include "tst_module.h"
-#include "tst_kconfig.h"
 
 #define MODULE_NAME	"init_module.ko"
 
@@ -28,12 +27,8 @@ static int sig_enforce;
 static void setup(void)
 {
 	int fd;
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
 
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
+	if (tst_module_signature_enforced())
 		sig_enforce = 1;
 
 	tst_module_exists(MODULE_NAME, NULL);
@@ -47,7 +42,6 @@ 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;
 	}
diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
index 3eb99574d9..c3979c08aa 100644
--- a/testcases/kernel/syscalls/init_module/init_module02.c
+++ b/testcases/kernel/syscalls/init_module/init_module02.c
@@ -17,7 +17,6 @@
 #include <stdlib.h>
 #include <errno.h>
 #include "lapi/init_module.h"
-#include "tst_kconfig.h"
 #include "tst_module.h"
 #include "tst_capability.h"
 
@@ -53,12 +52,8 @@ static void setup(void)
 {
 	struct stat sb;
 	int fd;
-	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
-	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
 
-	tst_kcmdline_parse(&params, 1);
-	tst_kconfig_read(&kconfig, 1);
-	if (params.found || kconfig.choice == 'y')
+	if (tst_module_signature_enforced())
 		sig_enforce = 1;
 
 	tst_module_exists(MODULE_NAME, NULL);
-- 
2.45.2


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

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

* [LTP] [PATCH 2/2] device-drivers: *module: Use helpers for module signature enforcement
  2024-12-27 16:00 [LTP] [PATCH 0/2] module signature enforcement helpers Petr Vorel
  2024-12-27 16:00 ` [LTP] [PATCH 1/2] lib: Add helpers for module signature enforcement Petr Vorel
  2024-12-27 16:00 ` [LTP] [PATCH 2/2] device-drivers: *module: Use " Petr Vorel
@ 2024-12-31  4:34 ` Po-Hsu Lin
  2024-12-31 11:50   ` Petr Vorel
  2 siblings, 1 reply; 5+ messages in thread
From: Po-Hsu Lin @ 2024-12-31  4:34 UTC (permalink / raw)
  To: ltp; +Cc: po-hsu.lin, rbm


On Sat, 28 Dec 2024 at 00:00, Petr Vorel <pvorel@suse.cz> wrote:
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  testcases/kernel/device-drivers/acpi/ltp_acpi.c       | 11 +----------
>  .../device-drivers/block/block_dev_user/block_dev.c   |  9 +--------
>  testcases/kernel/device-drivers/pci/tpci_user/tpci.c  | 11 +----------
>  testcases/kernel/device-drivers/uaccess/uaccess.c     | 11 +----------
>  testcases/kernel/firmware/fw_load_user/fw_load.c      | 10 +---------
>  .../kernel/syscalls/delete_module/delete_module01.c   |  9 +--------
>  .../kernel/syscalls/delete_module/delete_module03.c   |  9 +--------
>  .../kernel/syscalls/finit_module/finit_module01.c     | 11 +----------
>  .../kernel/syscalls/finit_module/finit_module02.c     |  7 +------
>  testcases/kernel/syscalls/init_module/init_module01.c |  8 +-------
>  testcases/kernel/syscalls/init_module/init_module02.c |  7 +------
>  11 files changed, 11 insertions(+), 92 deletions(-)
> 
> diff --git a/testcases/kernel/device-drivers/acpi/ltp_acpi.c b/testcases/kernel/device-drivers/acpi/ltp_acpi.c
> index c674b20321..eb5e197239 100644
> --- a/testcases/kernel/device-drivers/acpi/ltp_acpi.c
> +++ b/testcases/kernel/device-drivers/acpi/ltp_acpi.c
> @@ -22,7 +22,6 @@
>  #include <stdlib.h>
>
>  #include "test.h"
> -#include "tst_kconfig.h"
>  #include "old_module.h"
>  #include "safe_macros.h"
>
> @@ -129,20 +128,12 @@ static void test_run(void)
>  int main(int argc, char *argv[])
>  {
>  	int acpi_disabled;
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
>
>  	tst_parse_opts(argc, argv, NULL, NULL);
> -
>  	tst_require_root();
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
> -
>  	tst_sig(FORK, DEF_HANDLER, cleanup);
>
> +	tst_check_module_signature_enforced();

One question, should we put this tst_check_module_signature_enforced()
before or after tst_sig()? Asking because it's different in tpci.c

>  	tst_module_load(NULL, module_name, NULL);
>  	module_loaded = 1;
>
> diff --git a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> index 2c3ffd9203..98b786a90b 100644
> --- a/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> +++ b/testcases/kernel/device-drivers/block/block_dev_user/block_dev.c
> @@ -17,7 +17,6 @@
>  #include <unistd.h>
>  #include <string.h>
>  
> -#include "tst_kconfig.h"
>  #include "tst_test.h"
>  #include "tst_module.h"
>  
> @@ -42,13 +41,7 @@ static void cleanup(void)
>  
>  static void run(unsigned int n)
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brk(TCONF, "module signature is enforced, skip test");
> +	tst_check_module_signature_enforced();
>  
>  	/*
>  	 * test-cases #8 and #9 can crash the kernel.
> diff --git a/testcases/kernel/device-drivers/pci/tpci_user/tpci.c b/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
> index aa07fdb427..1bcc68c23c 100644
> --- a/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
> +++ b/testcases/kernel/device-drivers/pci/tpci_user/tpci.c
> @@ -27,7 +27,6 @@
>  #include <errno.h>
>  
>  #include "test.h"
> -#include "tst_kconfig.h"
>  #include "safe_macros.h"
>  #include "old_module.h"
>  
> @@ -50,16 +49,8 @@ static void cleanup(void)
>  
>  void setup(void)
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
>  	tst_require_root();
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
> -
> +	tst_check_module_signature_enforced();
>  	tst_sig(FORK, DEF_HANDLER, cleanup);
>  }
>  
> diff --git a/testcases/kernel/device-drivers/uaccess/uaccess.c b/testcases/kernel/device-drivers/uaccess/uaccess.c
> index c6dfeb8456..58bef13ef3 100644
> --- a/testcases/kernel/device-drivers/uaccess/uaccess.c
> +++ b/testcases/kernel/device-drivers/uaccess/uaccess.c
> @@ -27,7 +27,6 @@
>  #include <unistd.h>
>  
>  #include "test.h"
> -#include "tst_kconfig.h"
>  #include "old_module.h"
>  #include "safe_macros.h"
>  
> @@ -93,20 +92,12 @@ static void tc_write_userspace(void)
>  
>  int main(int argc, char *argv[])
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
>  	tst_parse_opts(argc, argv, NULL, NULL);
>  
>  	tst_require_root();
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
> -
>  	tst_sig(FORK, DEF_HANDLER, cleanup);
>  
> +	tst_check_module_signature_enforced();
>  	tst_module_load(NULL, module_name, NULL);
>  	module_loaded = 1;
>  
> diff --git a/testcases/kernel/firmware/fw_load_user/fw_load.c b/testcases/kernel/firmware/fw_load_user/fw_load.c
> index b34b56fae5..7ebb2cc8ed 100644
> --- a/testcases/kernel/firmware/fw_load_user/fw_load.c
> +++ b/testcases/kernel/firmware/fw_load_user/fw_load.c
> @@ -29,7 +29,6 @@
>  #include <string.h>
>  
>  #include "test.h"
> -#include "tst_kconfig.h"
>  #include "safe_macros.h"
>  #include "old_module.h"
>  
> @@ -103,9 +102,6 @@ static void help(void)
>  
>  void setup(int argc, char *argv[])
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
>  	tst_parse_opts(argc, argv, options, help);
>  
>  	if (nflag) {
> @@ -116,11 +112,7 @@ void setup(int argc, char *argv[])
>  	}
>  
>  	tst_require_root();
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
> +	tst_check_module_signature_enforced();
>  
>  	char fw_size_param[19];
>  	snprintf(fw_size_param, 19, "fw_size=%d", fw_size);
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module01.c b/testcases/kernel/syscalls/delete_module/delete_module01.c
> index 5ccbe6fd5b..48b324309f 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module01.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module01.c
> @@ -17,7 +17,6 @@
>  #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"
> @@ -27,13 +26,7 @@ static int module_loaded;
>  
>  static void do_delete_module(void)
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brk(TCONF, "module signature is enforced, skip test");
> +	tst_check_module_signature_enforced();
>  
>  	if (!module_loaded) {
>  		tst_module_load(MODULE_NAME_KO, NULL);
> diff --git a/testcases/kernel/syscalls/delete_module/delete_module03.c b/testcases/kernel/syscalls/delete_module/delete_module03.c
> index 76ccfb1e25..be358381ef 100644
> --- a/testcases/kernel/syscalls/delete_module/delete_module03.c
> +++ b/testcases/kernel/syscalls/delete_module/delete_module03.c
> @@ -16,7 +16,6 @@
>  #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"
> @@ -52,13 +51,7 @@ static void do_delete_module(void)
>  
>  static void setup(void)
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		tst_brk(TCONF, "module signature is enforced, skip test");
> +	tst_check_module_signature_enforced();
>  
>  	/* Load first kernel module */
>  	tst_module_load(DUMMY_MOD_KO, NULL);
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module01.c b/testcases/kernel/syscalls/finit_module/finit_module01.c
> index d75e5e4753..0ed68c7372 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module01.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
> @@ -17,7 +17,6 @@
>  #include <errno.h>
>  #include "lapi/init_module.h"
>  #include "tst_module.h"
> -#include "tst_kconfig.h"
>  
>  #define MODULE_NAME	"finit_module.ko"
>  
> @@ -27,14 +26,7 @@ static char *mod_path;
>  
>  static void setup(void)
>  {
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> -
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> -		sig_enforce = 1;

This test will expect an EKEYREJECTED error if the module signature is
enforced. So I think:

if (tst_module_signature_enforced())
	sig_enforce = 1;

And remove tst_check_module_signature_enforced() below, otherwise the
test will be skipped.

> -
> +	tst_check_module_signature_enforced();
>  	tst_module_exists(MODULE_NAME, &mod_path);
>  
>  	fd = SAFE_OPEN(mod_path, O_RDONLY|O_CLOEXEC);
> @@ -43,7 +35,6 @@ 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;
>  	}
> diff --git a/testcases/kernel/syscalls/finit_module/finit_module02.c b/testcases/kernel/syscalls/finit_module/finit_module02.c
> index dc81ae727d..e16164ee25 100644
> --- a/testcases/kernel/syscalls/finit_module/finit_module02.c
> +++ b/testcases/kernel/syscalls/finit_module/finit_module02.c
> @@ -18,7 +18,6 @@
>  #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"
> @@ -77,12 +76,8 @@ static struct tcase tcases[] = {
>  static void setup(void)
>  {
>  	unsigned long int i;
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
>  
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> +	if (tst_module_signature_enforced())
>  		sig_enforce = 1;
>  
>  	tst_module_exists(MODULE_NAME, &mod_path);
> diff --git a/testcases/kernel/syscalls/init_module/init_module01.c b/testcases/kernel/syscalls/init_module/init_module01.c
> index d7ed08b143..850f826e62 100644
> --- a/testcases/kernel/syscalls/init_module/init_module01.c
> +++ b/testcases/kernel/syscalls/init_module/init_module01.c
> @@ -17,7 +17,6 @@
>  #include <errno.h>
>  #include "lapi/init_module.h"
>  #include "tst_module.h"
> -#include "tst_kconfig.h"
>  
>  #define MODULE_NAME	"init_module.ko"
>  
> @@ -28,12 +27,8 @@ static int sig_enforce;
>  static void setup(void)
>  {
>  	int fd;
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
>  
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> +	if (tst_module_signature_enforced())
>  		sig_enforce = 1;
>  
>  	tst_module_exists(MODULE_NAME, NULL);
> @@ -47,7 +42,6 @@ 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;
>  	}
> diff --git a/testcases/kernel/syscalls/init_module/init_module02.c b/testcases/kernel/syscalls/init_module/init_module02.c
> index 3eb99574d9..c3979c08aa 100644
> --- a/testcases/kernel/syscalls/init_module/init_module02.c
> +++ b/testcases/kernel/syscalls/init_module/init_module02.c
> @@ -17,7 +17,6 @@
>  #include <stdlib.h>
>  #include <errno.h>
>  #include "lapi/init_module.h"
> -#include "tst_kconfig.h"
>  #include "tst_module.h"
>  #include "tst_capability.h"
>  
> @@ -53,12 +52,8 @@ static void setup(void)
>  {
>  	struct stat sb;
>  	int fd;
> -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
>  
> -	tst_kcmdline_parse(&params, 1);
> -	tst_kconfig_read(&kconfig, 1);
> -	if (params.found || kconfig.choice == 'y')
> +	if (tst_module_signature_enforced())
>  		sig_enforce = 1;
>  
>  	tst_module_exists(MODULE_NAME, NULL);
> -- 
> 2.45.2

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

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

* Re: [LTP] [PATCH 2/2] device-drivers: *module: Use helpers for module signature enforcement
  2024-12-31  4:34 ` Po-Hsu Lin
@ 2024-12-31 11:50   ` Petr Vorel
  0 siblings, 0 replies; 5+ messages in thread
From: Petr Vorel @ 2024-12-31 11:50 UTC (permalink / raw)
  To: Po-Hsu Lin; +Cc: rbm, ltp

Hi Po-Hsu Lin,

> > +++ b/testcases/kernel/device-drivers/acpi/ltp_acpi.c
> > @@ -22,7 +22,6 @@
> >  #include <stdlib.h>

> >  #include "test.h"
> > -#include "tst_kconfig.h"
> >  #include "old_module.h"
> >  #include "safe_macros.h"

> > @@ -129,20 +128,12 @@ static void test_run(void)
> >  int main(int argc, char *argv[])
> >  {
> >  	int acpi_disabled;
> > -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> > -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");

> >  	tst_parse_opts(argc, argv, NULL, NULL);
> > -
> >  	tst_require_root();
> > -
> > -	tst_kcmdline_parse(&params, 1);
> > -	tst_kconfig_read(&kconfig, 1);
> > -	if (params.found || kconfig.choice == 'y')
> > -		tst_brkm(TCONF, tst_exit, "module signature is enforced, skip test");
> > -
> >  	tst_sig(FORK, DEF_HANDLER, cleanup);

> > +	tst_check_module_signature_enforced();

> One question, should we put this tst_check_module_signature_enforced()
> before or after tst_sig()? Asking because it's different in tpci.c

I suppose either way is ok, because we don't call the cleanup function anyway.
Best would be to convert the test to the new LTP API.
But I'll move it in tpci.c below tst_sig().

...
> > +++ b/testcases/kernel/syscalls/finit_module/finit_module01.c
> > @@ -17,7 +17,6 @@
> >  #include <errno.h>
> >  #include "lapi/init_module.h"
> >  #include "tst_module.h"
> > -#include "tst_kconfig.h"

> >  #define MODULE_NAME	"finit_module.ko"

> > @@ -27,14 +26,7 @@ static char *mod_path;

> >  static void setup(void)
> >  {
> > -	struct tst_kcmdline_var params = TST_KCMDLINE_INIT("module.sig_enforce");
> > -	struct tst_kconfig_var kconfig = TST_KCONFIG_INIT("CONFIG_MODULE_SIG_FORCE");
> > -
> > -	tst_kcmdline_parse(&params, 1);
> > -	tst_kconfig_read(&kconfig, 1);
> > -	if (params.found || kconfig.choice == 'y')
> > -		sig_enforce = 1;

> This test will expect an EKEYREJECTED error if the module signature is
> enforced. So I think:

> if (tst_module_signature_enforced())
> 	sig_enforce = 1;

> And remove tst_check_module_signature_enforced() below, otherwise the
> test will be skipped.

+1. I have this in other tests, but obviously I omitted this in
finit_module01.c. I'll fix it in v2.

Thanks for your review!

Kind regards,
Petr

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

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

end of thread, other threads:[~2024-12-31 11:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-27 16:00 [LTP] [PATCH 0/2] module signature enforcement helpers Petr Vorel
2024-12-27 16:00 ` [LTP] [PATCH 1/2] lib: Add helpers for module signature enforcement Petr Vorel
2024-12-27 16:00 ` [LTP] [PATCH 2/2] device-drivers: *module: Use " Petr Vorel
2024-12-31  4:34 ` Po-Hsu Lin
2024-12-31 11:50   ` Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox