linux-kernel - [PATCH 4/5] module: merge remnants of setup_load

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: <20230319213542.1790479-5-mcgrof@kernel.org>
Date:   Sun, 19 Mar 2023 14:35:41 -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 4/5] module: merge remnants of setup_load_info() to elf validation

The setup_load_info() was actually had ELF validation checks of its
own. To later cache useful variables as an secondary step just means
looping again over the ELF sections we just validated. We can simply
keep tabs of the key sections of interest as we validate the module
ELF section in one swoop, so do that and merge the two routines
together.

Expand a bit on the documentation / intent / goals.

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

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 84a7f96cf35a..929644d79d38 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1647,12 +1647,26 @@ static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
 }
 
 /*
- * Sanity checks against invalid binaries, wrong arch, weird elf version.
+ * Check userspace passed ELF module against our expectations, and cache
+ * useful variables for further processing as we go.
  *
- * Also do basic validity checks against section offsets and sizes, the
+ * This does basic validity checks against section offsets and sizes, the
  * section name string table, and the indices used for it (sh_name).
+ *
+ * As a last step, since we're already checking the ELF sections we cache
+ * useful variables which will be used later for our convenience:
+ *
+ * 	o pointers to section headers
+ * 	o cache the modinfo symbol section
+ * 	o cache the string symbol section
+ * 	o cache the module section
+ *
+ * As a last step we set info->mod to the temporary copy of the module in
+ * info->hdr. The final one will be allocated in move_module(). Any
+ * modifications we make to our copy of the module will be carried over
+ * to the final minted module.
  */
-static int elf_validity_check(struct load_info *info)
+static int elf_validity_cache_copy(struct load_info *info, int flags)
 {
 	unsigned int i;
 	Elf_Shdr *shdr, *strhdr;
@@ -1872,6 +1886,13 @@ static int elf_validity_check(struct load_info *info)
 	if (!info->name)
 		info->name = info->mod->name;
 
+	if (flags & MODULE_INIT_IGNORE_MODVERSIONS)
+		info->index.vers = 0; /* Pretend no __versions section! */
+	else
+		info->index.vers = find_sec(info, "__versions");
+
+	info->index.pcpu = find_pcpusec(info);
+
 	return 0;
 
 no_exec:
@@ -1984,26 +2005,6 @@ static int rewrite_section_headers(struct load_info *info, int flags)
 	return 0;
 }
 
-/*
- * Set up our basic convenience variables (pointers to section headers,
- * search for module section index etc), and do some basic section
- * verification.
- *
- * Set info->mod to the temporary copy of the module in info->hdr. The final one
- * will be allocated in move_module().
- */
-static int setup_load_info(struct load_info *info, int flags)
-{
-	if (flags & MODULE_INIT_IGNORE_MODVERSIONS)
-		info->index.vers = 0; /* Pretend no __versions section! */
-	else
-		info->index.vers = find_sec(info, "__versions");
-
-	info->index.pcpu = find_pcpusec(info);
-
-	return 0;
-}
-
 /*
  * These calls taint the kernel depending certain module circumstances */
 static void module_augment_kernel_taints(struct module *mod, struct load_info *info)
@@ -2809,17 +2810,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 	/*
 	 * Do basic sanity checks against the ELF header and
-	 * sections.
-	 */
-	err = elf_validity_check(info);
-	if (err)
-		goto free_copy;
-
-	/*
-	 * Everything checks out, so set up the section info
-	 * in the info structure.
+	 * sections. Cache useful sections and set the
+	 * info->mod to the userspace passed struct module.
 	 */
-	err = setup_load_info(info, flags);
+	err = elf_validity_cache_copy(info, flags);
 	if (err)
 		goto free_copy;
 
-- 
2.39.1