| 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
| ||
|
Message-ID: <alpine.LFD.2.00.1006020102520.8175@i5.linux-foundation.org>
Date: Wed, 2 Jun 2010 01:12:32 -0700 (PDT)
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Rusty Russell <rusty@...tcorp.com.au>
cc: Brandon Philips <brandon@...p.org>,
Andrew Morton <akpm@...ux-foundation.org>,
"Rafael J. Wysocki" <rjw@...k.pl>,
LKML <linux-kernel@...r.kernel.org>,
Jon Masters <jonathan@...masters.org>,
Tejun Heo <htejun@...il.com>,
Masami Hiramatsu <mhiramat@...hat.com>,
Kay Sievers <kay.sievers@...y.org>
Subject: Re: [PATCH 2/2] module: fix bne2 "gave up waiting for init of module
libcrc32c"
On Wed, 2 Jun 2010, Linus Torvalds wrote:
>
> IOW, things like this.. Pure code movement to peel off some stuff.
Here's a second one. It's slightly less trivial - since we now have error
cases - and equally untested so it may well be totally broken. But it also
cleans up a bit more, and avoids one of the goto targets, because the
"move_module()" helper now does both allocations or none.
But now I'm bored and tired, so I think this will be all for tonight. The
load_module function has gone from ~490 lines to ~370 lines, but that's
still so many that I don't honestly think this makes any readability
improvement yet.
But add a few more helper functions (say, one for doing relocations, one
for doing some of the verification, one for doing the license and
tainting, one to trivially push the icache flush into its own helper etc),
and it probably will never fit in 50 lines, but maybe it could be 150.
Linus
---
kernel/module.c | 119 ++++++++++++++++++++++++++++++-------------------------
1 files changed, 65 insertions(+), 54 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 165d97e..72567e6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2170,6 +2170,67 @@ static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, Elf_Shdr *se
#endif
}
+static struct module *move_module(struct module *mod, Elf_Ehdr *hdr, Elf_Shdr *sechdrs, char *secstrings, unsigned modindex)
+{
+ int i;
+ void *ptr;
+
+ /* Do the allocs. */
+ ptr = module_alloc_update_bounds(mod->core_size);
+ /*
+ * The pointer to this block is stored in the module structure
+ * which is inside the block. Just mark it as not being a
+ * leak.
+ */
+ kmemleak_not_leak(ptr);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ memset(ptr, 0, mod->core_size);
+ mod->module_core = ptr;
+
+ ptr = module_alloc_update_bounds(mod->init_size);
+ /*
+ * The pointer to this block is stored in the module structure
+ * which is inside the block. This block doesn't need to be
+ * scanned as it contains data and code that will be freed
+ * after the module is initialized.
+ */
+ kmemleak_ignore(ptr);
+ if (!ptr && mod->init_size) {
+ module_free(mod, mod->module_core);
+ return ERR_PTR(-ENOMEM);
+ }
+ memset(ptr, 0, mod->init_size);
+ mod->module_init = ptr;
+
+ /* Transfer each section which specifies SHF_ALLOC */
+ DEBUGP("final section addresses:\n");
+ for (i = 0; i < hdr->e_shnum; i++) {
+ void *dest;
+
+ if (!(sechdrs[i].sh_flags & SHF_ALLOC))
+ continue;
+
+ if (sechdrs[i].sh_entsize & INIT_OFFSET_MASK)
+ dest = mod->module_init
+ + (sechdrs[i].sh_entsize & ~INIT_OFFSET_MASK);
+ else
+ dest = mod->module_core + sechdrs[i].sh_entsize;
+
+ if (sechdrs[i].sh_type != SHT_NOBITS)
+ memcpy(dest, (void *)sechdrs[i].sh_addr,
+ sechdrs[i].sh_size);
+ /* Update sh_addr to point to copy in image. */
+ sechdrs[i].sh_addr = (unsigned long)dest;
+ DEBUGP("\t0x%lx %s\n", sechdrs[i].sh_addr, secstrings + sechdrs[i].sh_name);
+ }
+ /* Module has been moved. */
+ mod = (void *)sechdrs[modindex].sh_addr;
+ kmemleak_load_module(mod, hdr, sechdrs, secstrings);
+ return mod;
+}
+
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static noinline struct module *load_module(void __user *umod,
@@ -2186,7 +2247,6 @@ static noinline struct module *load_module(void __user *umod,
unsigned int modindex, versindex, infoindex, pcpuindex;
struct module *mod;
long err = 0;
- void *ptr = NULL; /* Stops spurious gcc warning */
unsigned long symoffs, stroffs, *strmap;
void __percpu *percpu;
@@ -2338,60 +2398,12 @@ static noinline struct module *load_module(void __user *umod,
symoffs = layout_symtab(mod, sechdrs, symindex, strindex, hdr,
secstrings, &stroffs, strmap);
- /* Do the allocs. */
- ptr = module_alloc_update_bounds(mod->core_size);
- /*
- * The pointer to this block is stored in the module structure
- * which is inside the block. Just mark it as not being a
- * leak.
- */
- kmemleak_not_leak(ptr);
- if (!ptr) {
- err = -ENOMEM;
+ /* Allocate and move to the final place */
+ mod = move_module(mod, hdr, sechdrs, secstrings, modindex);
+ if (IS_ERR(mod)) {
+ err = PTR_ERR(mod);
goto free_percpu;
}
- memset(ptr, 0, mod->core_size);
- mod->module_core = ptr;
-
- ptr = module_alloc_update_bounds(mod->init_size);
- /*
- * The pointer to this block is stored in the module structure
- * which is inside the block. This block doesn't need to be
- * scanned as it contains data and code that will be freed
- * after the module is initialized.
- */
- kmemleak_ignore(ptr);
- if (!ptr && mod->init_size) {
- err = -ENOMEM;
- goto free_core;
- }
- memset(ptr, 0, mod->init_size);
- mod->module_init = ptr;
-
- /* Transfer each section which specifies SHF_ALLOC */
- DEBUGP("final section addresses:\n");
- for (i = 0; i < hdr->e_shnum; i++) {
- void *dest;
-
- if (!(sechdrs[i].sh_flags & SHF_ALLOC))
- continue;
-
- if (sechdrs[i].sh_entsize & INIT_OFFSET_MASK)
- dest = mod->module_init
- + (sechdrs[i].sh_entsize & ~INIT_OFFSET_MASK);
- else
- dest = mod->module_core + sechdrs[i].sh_entsize;
-
- if (sechdrs[i].sh_type != SHT_NOBITS)
- memcpy(dest, (void *)sechdrs[i].sh_addr,
- sechdrs[i].sh_size);
- /* Update sh_addr to point to copy in image. */
- sechdrs[i].sh_addr = (unsigned long)dest;
- DEBUGP("\t0x%lx %s\n", sechdrs[i].sh_addr, secstrings + sechdrs[i].sh_name);
- }
- /* Module has been moved. */
- mod = (void *)sechdrs[modindex].sh_addr;
- kmemleak_load_module(mod, hdr, sechdrs, secstrings);
#if defined(CONFIG_MODULE_UNLOAD)
mod->refptr = alloc_percpu(struct module_ref);
@@ -2577,7 +2589,6 @@ static noinline struct module *load_module(void __user *umod,
free_init:
#endif
module_free(mod, mod->module_init);
- free_core:
module_free(mod, mod->module_core);
/* mod will be freed with core. Don't access it beyond this line! */
free_percpu:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists