linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luis Chamberlain <mcgrof@kernel.org>
To: linux-modules@vger.kernel.org, linux-kernel@vger.kernel.org,
	pmladek@suse.com, david@redhat.com, petr.pavlu@suse.com,
	prarit@redhat.com
Cc: christophe.leroy@csgroup.eu, song@kernel.org, mcgrof@kernel.org
Subject: [PATCH 08/12] module: split taint adding with info checking
Date: Sun, 19 Mar 2023 14:27:42 -0700	[thread overview]
Message-ID: <20230319212746.1783033-9-mcgrof@kernel.org> (raw)
In-Reply-To: <20230319212746.1783033-1-mcgrof@kernel.org>

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@kernel.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


  parent reply	other threads:[~2023-03-19 21:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-19 21:27 [PATCH 00/12] module: cleanup and call taints after is inserted Luis Chamberlain
2023-03-19 21:27 ` [PATCH 01/12] module: move get_modinfo() helpers all above Luis Chamberlain
2023-03-19 21:27 ` [PATCH 02/12] module: rename next_string() to module_next_tag_pair() Luis Chamberlain
2023-03-19 21:27 ` [PATCH 03/12] module: add a for_each_modinfo_entry() Luis Chamberlain
2023-03-19 21:27 ` [PATCH 04/12] module: move early sanity checks into a helper Luis Chamberlain
2023-03-24 13:02   ` Petr Pavlu
2023-03-24 18:33     ` Luis Chamberlain
2023-03-19 21:27 ` [PATCH 05/12] module: move check_modinfo() early to early_mod_check() Luis Chamberlain
2023-03-19 21:27 ` [PATCH 06/12] module: rename set_license() to module_license_taint_check() Luis Chamberlain
2023-03-19 21:27 ` [PATCH 07/12] module: split taint work out of check_modinfo_livepatch() Luis Chamberlain
2023-03-19 21:27 ` Luis Chamberlain [this message]
2023-03-19 21:27 ` [PATCH 09/12] module: move tainting until after a module hits our linked list Luis Chamberlain
2023-03-19 21:27 ` [PATCH 10/12] module: move signature taint to module_augment_kernel_taints() Luis Chamberlain
2023-03-19 21:27 ` [PATCH 11/12] module: converge taint work together Luis Chamberlain
2023-03-19 21:27 ` [PATCH 12/12] module: rename check_module_license_and_versions() to check_export_symbol_versions() Luis Chamberlain
2023-03-22 23:42 ` [PATCH 00/12] module: cleanup and call taints after is inserted Luis Chamberlain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230319212746.1783033-9-mcgrof@kernel.org \
    --to=mcgrof@kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=petr.pavlu@suse.com \
    --cc=pmladek@suse.com \
    --cc=prarit@redhat.com \
    --cc=song@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).