linux-kernel - [PATCH] module: Taint the kernel when write-protecting ro_after

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250306103712.29549-1-petr.pavlu@suse.com>
Date: Thu,  6 Mar 2025 11:36:55 +0100
From: Petr Pavlu <petr.pavlu@...e.com>
To: Luis Chamberlain <mcgrof@...nel.org>,
	Petr Pavlu <petr.pavlu@...e.com>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	Daniel Gomez <da.gomez@...sung.com>,
	Kees Cook <kees@...nel.org>,
	Petr Mladek <pmladek@...e.com>
Cc: Jani Nikula <jani.nikula@...el.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	John Ogness <john.ogness@...utronix.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-modules@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] module: Taint the kernel when write-protecting ro_after_init fails

In the unlikely case that setting ro_after_init data to read-only fails, it
is too late to cancel loading of the module. The loader then issues only
a warning about the situation. Given that this reduces the kernel's
protection, it was suggested to make the failure more visible by tainting
the kernel.

Allow TAINT_BAD_PAGE to be set per-module and use it in this case. The flag
is set in similar situations and has the following description in
Documentation/admin-guide/tainted-kernels.rst: "bad page referenced or some
unexpected page flags".

Adjust the warning that reports the failure to avoid references to internal
functions and to add information about the kernel being tainted, both to
match the style of other messages in the file. Additionally, merge the
message on a single line because checkpatch.pl recommends that for the
ability to grep for the string.

Suggested-by: Kees Cook <kees@...nel.org>
Signed-off-by: Petr Pavlu <petr.pavlu@...e.com>
---
I opted to use TAINT_BAD_PAGE for now because it seemed unnecessary to me
to introduce a new flag only for this specific case. However, if we end up
similarly checking set_memory_*() in the boot context, a separate flag
would be probably better.
---
 kernel/module/main.c | 7 ++++---
 kernel/panic.c       | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 1fb9ad289a6f..8f424a107b92 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3030,10 +3030,11 @@ static noinline int do_init_module(struct module *mod)
 	rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms);
 #endif
 	ret = module_enable_rodata_ro_after_init(mod);
-	if (ret)
-		pr_warn("%s: module_enable_rodata_ro_after_init() returned %d, "
-			"ro_after_init data might still be writable\n",
+	if (ret) {
+		pr_warn("%s: write-protecting ro_after_init data failed with %d, the data might still be writable - tainting kernel\n",
 			mod->name, ret);
+		add_taint_module(mod, TAINT_BAD_PAGE, LOCKDEP_STILL_OK);
+	}
 
 	mod_tree_remove_init(mod);
 	module_arch_freeing_init(mod);
diff --git a/kernel/panic.c b/kernel/panic.c
index d8635d5cecb2..794c443bfb5c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -497,7 +497,7 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
 	TAINT_FLAG(CPU_OUT_OF_SPEC,		'S', ' ', false),
 	TAINT_FLAG(FORCED_RMMOD,		'R', ' ', false),
 	TAINT_FLAG(MACHINE_CHECK,		'M', ' ', false),
-	TAINT_FLAG(BAD_PAGE,			'B', ' ', false),
+	TAINT_FLAG(BAD_PAGE,			'B', ' ', true),
 	TAINT_FLAG(USER,			'U', ' ', false),
 	TAINT_FLAG(DIE,				'D', ' ', false),
 	TAINT_FLAG(OVERRIDDEN_ACPI_TABLE,	'A', ' ', false),

base-commit: 48a5eed9ad584315c30ed35204510536235ce402
-- 
2.43.0