linux-kernel - [PATCH 08/12] module: split taint adding with info checking

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230319212746.1783033-9-mcgrof@kernel.org>
Date:   Sun, 19 Mar 2023 14:27:42 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
        pmladek@...e.com, david@...hat.com, petr.pavlu@...e.com,
        prarit@...hat.com
Cc:     christophe.leroy@...roup.eu, song@...nel.org, mcgrof@...nel.org
Subject: [PATCH 08/12] module: split taint adding with info checking

check_modinfo() actually does two things:

 a) sanity checks, some of which are fatal, and so we
    prevent the user from completing trying to load a module
 b) taints the kernel

The taints are pretty heavy handed because we're tainting the kernel
*before* we ever even get to load the module into the modules linked
list. That is, it it can fail for other reasons later as we review the
module's structure.

But this commit makes no functional changes, it just makes the intent
clearer and splits the code up where needed to make that happen.

Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
---
 kernel/module/main.c | 62 ++++++++++++++++++++++++++++----------------
 1 file changed, 40 insertions(+), 22 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index cfb2ff5185fe..a3953ca18090 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1951,25 +1951,10 @@ static int setup_load_info(struct load_info *info, int flags)
 	return 0;
 }
 
-static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+/*
+ * These calls taint the kernel depending certain module circumstances */
+static void module_augment_kernel_taints(struct module *mod, struct load_info *info)
 {
-	const char *modmagic = get_modinfo(info, "vermagic");
-	int err;
-
-	if (flags & MODULE_INIT_IGNORE_VERMAGIC)
-		modmagic = NULL;
-
-	/* This is allowed: modprobe --force will invalidate it. */
-	if (!modmagic) {
-		err = try_to_force_load(mod, "bad vermagic");
-		if (err)
-			return err;
-	} else if (!same_magic(modmagic, vermagic, info->index.vers)) {
-		pr_err("%s: version magic '%s' should be '%s'\n",
-		       info->name, modmagic, vermagic);
-		return -ENOEXEC;
-	}
-
 	if (!get_modinfo(info, "intree")) {
 		if (!test_taint(TAINT_OOT_MODULE))
 			pr_warn("%s: loading out-of-tree module taints kernel.\n",
@@ -1985,15 +1970,12 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 			"is unknown, you have been warned.\n", mod->name);
 	}
 
-	err = check_modinfo_livepatch(mod, info);
-	if (err)
-		return err;
-
 	if (is_livepatch_module(mod)) {
 		add_taint_module(mod, TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
 		pr_notice_once("%s: tainting kernel with TAINT_LIVEPATCH\n",
 				mod->name);
 	}
+
 	module_license_taint_check(mod, get_modinfo(info, "license"));
 
 	if (get_modinfo(info, "test")) {
@@ -2002,6 +1984,42 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 				mod->name);
 		add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK);
 	}
+}
+
+static int check_modinfo(struct module *mod, struct load_info *info, int flags)
+{
+	const char *modmagic = get_modinfo(info, "vermagic");
+	int err;
+
+	if (flags & MODULE_INIT_IGNORE_VERMAGIC)
+		modmagic = NULL;
+
+	/* This is allowed: modprobe --force will invalidate it. */
+	if (!modmagic) {
+		err = try_to_force_load(mod, "bad vermagic");
+		if (err)
+			return err;
+	} else if (!same_magic(modmagic, vermagic, info->index.vers)) {
+		pr_err("%s: version magic '%s' should be '%s'\n",
+		       info->name, modmagic, vermagic);
+		return -ENOEXEC;
+	}
+
+	err = check_modinfo_livepatch(mod, info);
+	if (err)
+		return err;
+
+	/*
+	 * We are tainting your kernel *even* if you try to load
+	 * modules with possible taints and we fail to load these
+	 * modules for other reasons.
+	 *
+	 * We have a descrepancy though, see the other taints for
+	 * signature and those in check_module_license_and_versions().
+	 *
+	 * We should compromise and converge.
+	 */
+	module_augment_kernel_taints(mod, info);
 
 	return 0;
 }
-- 
2.39.1