From 2409e74278b7fb917d39ef6d3c16223c04a386f2 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Thu, 5 Aug 2010 12:59:00 -0600 Subject: module: module_unload_init() cleanup No need to clear mod->refptr in module_unload_init(), since alloc_percpu() already clears allocated chunks. Signed-off-by: Eric Dumazet Signed-off-by: Rusty Russell (removed unused var) diff --git a/kernel/module.c b/kernel/module.c index 6c56282..404e722 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -526,14 +526,8 @@ EXPORT_TRACEPOINT_SYMBOL(module_get); /* Init the unload section of the module. */ static void module_unload_init(struct module *mod) { - int cpu; - INIT_LIST_HEAD(&mod->source_list); INIT_LIST_HEAD(&mod->target_list); - for_each_possible_cpu(cpu) { - per_cpu_ptr(mod->refptr, cpu)->incs = 0; - per_cpu_ptr(mod->refptr, cpu)->decs = 0; - } /* Hold reference count during initialization. */ __this_cpu_write(mod->refptr->incs, 1); -- cgit v0.10.2 From f91a13bb99b73961d4e2743a6ff296ac553abc4f Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 5 Aug 2010 12:59:02 -0600 Subject: module: refactor load_module I'd start from the trivial stuff. There's a fair amount of straight-line code that just makes the function hard to read just because you have to page up and down so far. Some of it is trivial to just create a helper function for. Signed-off-by: Linus Torvalds Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 404e722..12905ed 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2107,6 +2107,71 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #endif +static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, + Elf_Shdr *sechdrs, const char *secstrings) +{ + mod->kp = section_objs(hdr, sechdrs, secstrings, "__param", + sizeof(*mod->kp), &mod->num_kp); + mod->syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab", + sizeof(*mod->syms), &mod->num_syms); + mod->crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab"); + mod->gpl_syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab_gpl", + sizeof(*mod->gpl_syms), + &mod->num_gpl_syms); + mod->gpl_crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab_gpl"); + mod->gpl_future_syms = section_objs(hdr, sechdrs, secstrings, + "__ksymtab_gpl_future", + sizeof(*mod->gpl_future_syms), + &mod->num_gpl_future_syms); + mod->gpl_future_crcs = section_addr(hdr, sechdrs, secstrings, + "__kcrctab_gpl_future"); + +#ifdef CONFIG_UNUSED_SYMBOLS + mod->unused_syms = section_objs(hdr, sechdrs, secstrings, + "__ksymtab_unused", + sizeof(*mod->unused_syms), + &mod->num_unused_syms); + mod->unused_crcs = section_addr(hdr, sechdrs, secstrings, + "__kcrctab_unused"); + mod->unused_gpl_syms = section_objs(hdr, sechdrs, secstrings, + "__ksymtab_unused_gpl", + sizeof(*mod->unused_gpl_syms), + &mod->num_unused_gpl_syms); + mod->unused_gpl_crcs = section_addr(hdr, sechdrs, secstrings, + "__kcrctab_unused_gpl"); +#endif +#ifdef CONFIG_CONSTRUCTORS + mod->ctors = section_objs(hdr, sechdrs, secstrings, ".ctors", + sizeof(*mod->ctors), &mod->num_ctors); +#endif + +#ifdef CONFIG_TRACEPOINTS + mod->tracepoints = section_objs(hdr, sechdrs, secstrings, + "__tracepoints", + sizeof(*mod->tracepoints), + &mod->num_tracepoints); +#endif +#ifdef CONFIG_EVENT_TRACING + mod->trace_events = section_objs(hdr, sechdrs, secstrings, + "_ftrace_events", + sizeof(*mod->trace_events), + &mod->num_trace_events); + /* + * This section contains pointers to allocated objects in the trace + * code and not scanning it leads to false positives. + */ + kmemleak_scan_area(mod->trace_events, sizeof(*mod->trace_events) * + mod->num_trace_events, GFP_KERNEL); +#endif +#ifdef CONFIG_FTRACE_MCOUNT_RECORD + /* sechdrs[0].sh_size is always zero */ + mod->ftrace_callsites = section_objs(hdr, sechdrs, secstrings, + "__mcount_loc", + sizeof(*mod->ftrace_callsites), + &mod->num_ftrace_callsites); +#endif +} + /* 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, @@ -2147,7 +2212,7 @@ static noinline struct module *load_module(void __user *umod, } /* Sanity checks against insmoding binaries or wrong arch, - weird elf version */ + weird elf version */ if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0 || hdr->e_type != ET_REL || !elf_check_arch(hdr) @@ -2326,7 +2391,8 @@ static noinline struct module *load_module(void __user *umod, 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); + DEBUGP("\t0x%lx %s\n", + sechdrs[i].sh_addr, secstrings + sechdrs[i].sh_name); } /* Module has been moved. */ mod = (void *)sechdrs[modindex].sh_addr; @@ -2368,66 +2434,8 @@ static noinline struct module *load_module(void __user *umod, /* Now we've got everything in the final locations, we can * find optional sections. */ - mod->kp = section_objs(hdr, sechdrs, secstrings, "__param", - sizeof(*mod->kp), &mod->num_kp); - mod->syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab", - sizeof(*mod->syms), &mod->num_syms); - mod->crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab"); - mod->gpl_syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab_gpl", - sizeof(*mod->gpl_syms), - &mod->num_gpl_syms); - mod->gpl_crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab_gpl"); - mod->gpl_future_syms = section_objs(hdr, sechdrs, secstrings, - "__ksymtab_gpl_future", - sizeof(*mod->gpl_future_syms), - &mod->num_gpl_future_syms); - mod->gpl_future_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_gpl_future"); + find_module_sections(mod, hdr, sechdrs, secstrings); -#ifdef CONFIG_UNUSED_SYMBOLS - mod->unused_syms = section_objs(hdr, sechdrs, secstrings, - "__ksymtab_unused", - sizeof(*mod->unused_syms), - &mod->num_unused_syms); - mod->unused_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_unused"); - mod->unused_gpl_syms = section_objs(hdr, sechdrs, secstrings, - "__ksymtab_unused_gpl", - sizeof(*mod->unused_gpl_syms), - &mod->num_unused_gpl_syms); - mod->unused_gpl_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_unused_gpl"); -#endif -#ifdef CONFIG_CONSTRUCTORS - mod->ctors = section_objs(hdr, sechdrs, secstrings, ".ctors", - sizeof(*mod->ctors), &mod->num_ctors); -#endif - -#ifdef CONFIG_TRACEPOINTS - mod->tracepoints = section_objs(hdr, sechdrs, secstrings, - "__tracepoints", - sizeof(*mod->tracepoints), - &mod->num_tracepoints); -#endif -#ifdef CONFIG_EVENT_TRACING - mod->trace_events = section_objs(hdr, sechdrs, secstrings, - "_ftrace_events", - sizeof(*mod->trace_events), - &mod->num_trace_events); - /* - * This section contains pointers to allocated objects in the trace - * code and not scanning it leads to false positives. - */ - kmemleak_scan_area(mod->trace_events, sizeof(*mod->trace_events) * - mod->num_trace_events, GFP_KERNEL); -#endif -#ifdef CONFIG_FTRACE_MCOUNT_RECORD - /* sechdrs[0].sh_size is always zero */ - mod->ftrace_callsites = section_objs(hdr, sechdrs, secstrings, - "__mcount_loc", - sizeof(*mod->ftrace_callsites), - &mod->num_ftrace_callsites); -#endif #ifdef CONFIG_MODVERSIONS if ((mod->num_syms && !mod->crcs) || (mod->num_gpl_syms && !mod->gpl_crcs) -- cgit v0.10.2 From 65b8a9b4d5525348e55cf63a43517610ee8e0970 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 5 Aug 2010 12:59:02 -0600 Subject: module: refactor load_module part 2 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. Signed-off-by: Linus Torvalds Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 12905ed..19ddcb0 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2082,7 +2082,8 @@ static void *module_alloc_update_bounds(unsigned long size) #ifdef CONFIG_DEBUG_KMEMLEAK static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, char *secstrings) + const Elf_Shdr *sechdrs, + const char *secstrings) { unsigned int i; @@ -2102,7 +2103,8 @@ static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #else static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, char *secstrings) + Elf_Shdr *sechdrs, + const char *secstrings) { } #endif @@ -2172,6 +2174,70 @@ static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, #endif } +static struct module *move_module(struct module *mod, + Elf_Ehdr *hdr, Elf_Shdr *sechdrs, + const 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, @@ -2188,7 +2254,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; struct _ddebug *debug = NULL; @@ -2342,61 +2407,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); @@ -2580,7 +2596,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: -- cgit v0.10.2 From 40dd2560ec2df21036db9ec8b971f92d98fd1969 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:03 -0600 Subject: module: refactor load_module part 3 Extract out the allocation and copying in from userspace, and the first set of modinfo checks. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 19ddcb0..d8faf35 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1803,7 +1803,7 @@ static char *next_string(char *string, unsigned long *secsize) return string; } -static char *get_modinfo(Elf_Shdr *sechdrs, +static char *get_modinfo(const Elf_Shdr *sechdrs, unsigned int info, const char *tag) { @@ -2109,6 +2109,73 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #endif +static int copy_and_check(Elf_Ehdr **hdrp, + const void __user *umod, unsigned long len) +{ + int err; + Elf_Ehdr *hdr; + + if (len < sizeof(*hdr)) + return -ENOEXEC; + + /* Suck in entire file: we'll want most of it. */ + /* vmalloc barfs on "unusual" numbers. Check here */ + if (len > 64 * 1024 * 1024 || (hdr = *hdrp = vmalloc(len)) == NULL) + return -ENOMEM; + + if (copy_from_user(hdr, umod, len) != 0) { + err = -EFAULT; + goto free_hdr; + } + + /* Sanity checks against insmoding binaries or wrong arch, + weird elf version */ + if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0 + || hdr->e_type != ET_REL + || !elf_check_arch(hdr) + || hdr->e_shentsize != sizeof(Elf_Shdr)) { + err = -ENOEXEC; + goto free_hdr; + } + + if (len < hdr->e_shoff + hdr->e_shnum * sizeof(Elf_Shdr)) { + err = -ENOEXEC; + goto free_hdr; + } + return 0; + +free_hdr: + vfree(hdr); + return err; +} + +static int check_modinfo(struct module *mod, + const Elf_Shdr *sechdrs, + unsigned int infoindex, unsigned int versindex) +{ + const char *modmagic = get_modinfo(sechdrs, infoindex, "vermagic"); + int err; + + /* 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, versindex)) { + printk(KERN_ERR "%s: version magic '%s' should be '%s'\n", + mod->name, modmagic, vermagic); + return -ENOEXEC; + } + + if (get_modinfo(sechdrs, infoindex, "staging")) { + add_taint_module(mod, TAINT_CRAP); + printk(KERN_WARNING "%s: module is from the staging directory," + " the quality is unknown, you have been warned.\n", + mod->name); + } + return 0; +} + static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, Elf_Shdr *sechdrs, const char *secstrings) { @@ -2246,14 +2313,13 @@ static noinline struct module *load_module(void __user *umod, { Elf_Ehdr *hdr; Elf_Shdr *sechdrs; - char *secstrings, *args, *modmagic, *strtab = NULL; - char *staging; + char *secstrings, *args, *strtab = NULL; unsigned int i; unsigned int symindex = 0; unsigned int strindex = 0; unsigned int modindex, versindex, infoindex, pcpuindex; struct module *mod; - long err = 0; + long err; unsigned long symoffs, stroffs, *strmap; void __percpu *percpu; struct _ddebug *debug = NULL; @@ -2263,31 +2329,10 @@ static noinline struct module *load_module(void __user *umod, DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", umod, len, uargs); - if (len < sizeof(*hdr)) - return ERR_PTR(-ENOEXEC); - - /* Suck in entire file: we'll want most of it. */ - /* vmalloc barfs on "unusual" numbers. Check here */ - if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) - return ERR_PTR(-ENOMEM); - - if (copy_from_user(hdr, umod, len) != 0) { - err = -EFAULT; - goto free_hdr; - } - - /* Sanity checks against insmoding binaries or wrong arch, - weird elf version */ - if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0 - || hdr->e_type != ET_REL - || !elf_check_arch(hdr) - || hdr->e_shentsize != sizeof(*sechdrs)) { - err = -ENOEXEC; - goto free_hdr; - } - if (len < hdr->e_shoff + hdr->e_shnum * sizeof(Elf_Shdr)) - goto truncated; + err = copy_and_check(&hdr, umod, len); + if (err) + return ERR_PTR(err); /* Convenience variables */ sechdrs = (void *)hdr + hdr->e_shoff; @@ -2347,26 +2392,9 @@ static noinline struct module *load_module(void __user *umod, goto free_hdr; } - modmagic = get_modinfo(sechdrs, infoindex, "vermagic"); - /* This is allowed: modprobe --force will invalidate it. */ - if (!modmagic) { - err = try_to_force_load(mod, "bad vermagic"); - if (err) - goto free_hdr; - } else if (!same_magic(modmagic, vermagic, versindex)) { - printk(KERN_ERR "%s: version magic '%s' should be '%s'\n", - mod->name, modmagic, vermagic); - err = -ENOEXEC; + err = check_modinfo(mod, sechdrs, infoindex, versindex); + if (err) goto free_hdr; - } - - staging = get_modinfo(sechdrs, infoindex, "staging"); - if (staging) { - add_taint_module(mod, TAINT_CRAP); - printk(KERN_WARNING "%s: module is from the staging directory," - " the quality is unknown, you have been warned.\n", - mod->name); - } /* Now copy in args */ args = strndup_user(uargs, ~0UL >> 1); -- cgit v0.10.2 From 9f85a4bbb1cf56f65b3d065a5f88204a757f2325 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:04 -0600 Subject: module: refactor load_module part 4 Allocate references inside module_unload_init(), clean up inside module_unload_free(). This version fixed to do allocation before __this_cpu_write, thanks to bug reports from linux-next from Dave Young and Stephen Rothwell . Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index d8faf35..241bc41 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -524,8 +524,12 @@ static char last_unloaded_module[MODULE_NAME_LEN+1]; EXPORT_TRACEPOINT_SYMBOL(module_get); /* Init the unload section of the module. */ -static void module_unload_init(struct module *mod) +static int module_unload_init(struct module *mod) { + mod->refptr = alloc_percpu(struct module_ref); + if (!mod->refptr) + return -ENOMEM; + INIT_LIST_HEAD(&mod->source_list); INIT_LIST_HEAD(&mod->target_list); @@ -533,6 +537,8 @@ static void module_unload_init(struct module *mod) __this_cpu_write(mod->refptr->incs, 1); /* Backwards compatibility macros put refcount during init. */ mod->waiter = current; + + return 0; } /* Does a already use b? */ @@ -612,6 +618,8 @@ static void module_unload_free(struct module *mod) kfree(use); } mutex_unlock(&module_mutex); + + free_percpu(mod->refptr); } #ifdef CONFIG_MODULE_FORCE_UNLOAD @@ -885,8 +893,9 @@ int ref_module(struct module *a, struct module *b) } EXPORT_SYMBOL_GPL(ref_module); -static inline void module_unload_init(struct module *mod) +static inline int module_unload_init(struct module *mod) { + return 0; } #endif /* CONFIG_MODULE_UNLOAD */ @@ -1559,10 +1568,7 @@ static void free_module(struct module *mod) module_free(mod, mod->module_init); kfree(mod->args); percpu_modfree(mod); -#if defined(CONFIG_MODULE_UNLOAD) - if (mod->refptr) - free_percpu(mod->refptr); -#endif + /* Free lock-classes: */ lockdep_free_key_range(mod->module_core, mod->core_size); @@ -2442,15 +2448,10 @@ static noinline struct module *load_module(void __user *umod, goto free_percpu; } -#if defined(CONFIG_MODULE_UNLOAD) - mod->refptr = alloc_percpu(struct module_ref); - if (!mod->refptr) { - err = -ENOMEM; - goto free_init; - } -#endif /* Now we've moved module, initialize linked lists, etc. */ - module_unload_init(mod); + err = module_unload_init(mod); + if (err) + goto free_init; /* Set up license info based on the info section */ set_license(mod, get_modinfo(sechdrs, infoindex, "license")); @@ -2619,10 +2620,7 @@ static noinline struct module *load_module(void __user *umod, cleanup: free_modinfo(mod); module_unload_free(mod); -#if defined(CONFIG_MODULE_UNLOAD) - free_percpu(mod->refptr); free_init: -#endif module_free(mod, mod->module_init); module_free(mod, mod->module_core); /* mod will be freed with core. Don't access it beyond this line! */ -- cgit v0.10.2 From 22e268ebecc549f1b1907f114cb44d6044bdee3c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:05 -0600 Subject: module: refactor load_module part 5 1) Extract out the relocation loop into apply_relocations 2) Extract license and version checks into check_module_license_and_versions 3) Extract icache flushing into flush_module_icache 4) Move __obsparm warning into find_module_sections 5) Move license setting into check_modinfo. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 241bc41..b536db8 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1698,6 +1698,39 @@ static int simplify_symbols(Elf_Shdr *sechdrs, return ret; } +static int apply_relocations(struct module *mod, + Elf_Ehdr *hdr, + Elf_Shdr *sechdrs, + unsigned int symindex, + unsigned int strindex) +{ + unsigned int i; + int err = 0; + + /* Now do relocations. */ + for (i = 1; i < hdr->e_shnum; i++) { + const char *strtab = (char *)sechdrs[strindex].sh_addr; + unsigned int info = sechdrs[i].sh_info; + + /* Not a valid relocation section? */ + if (info >= hdr->e_shnum) + continue; + + /* Don't bother with non-allocated sections */ + if (!(sechdrs[info].sh_flags & SHF_ALLOC)) + continue; + + if (sechdrs[i].sh_type == SHT_REL) + err = apply_relocate(sechdrs, strtab, symindex, i, mod); + else if (sechdrs[i].sh_type == SHT_RELA) + err = apply_relocate_add(sechdrs, strtab, symindex, i, + mod); + if (err < 0) + break; + } + return err; +} + /* Additional bytes needed by arch in front of individual sections */ unsigned int __weak arch_mod_section_prepend(struct module *mod, unsigned int section) @@ -2179,6 +2212,10 @@ static int check_modinfo(struct module *mod, " the quality is unknown, you have been warned.\n", mod->name); } + + /* Set up license info based on the info section */ + set_license(mod, get_modinfo(sechdrs, infoindex, "license")); + return 0; } @@ -2245,6 +2282,10 @@ static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, sizeof(*mod->ftrace_callsites), &mod->num_ftrace_callsites); #endif + + if (section_addr(hdr, sechdrs, secstrings, "__obsparm")) + printk(KERN_WARNING "%s: Ignoring obsolete parameters\n", + mod->name); } static struct module *move_module(struct module *mod, @@ -2311,6 +2352,60 @@ static struct module *move_module(struct module *mod, return mod; } +static int check_module_license_and_versions(struct module *mod, + Elf_Shdr *sechdrs) +{ + /* + * ndiswrapper is under GPL by itself, but loads proprietary modules. + * Don't use add_taint_module(), as it would prevent ndiswrapper from + * using GPL-only symbols it needs. + */ + if (strcmp(mod->name, "ndiswrapper") == 0) + add_taint(TAINT_PROPRIETARY_MODULE); + + /* driverloader was caught wrongly pretending to be under GPL */ + if (strcmp(mod->name, "driverloader") == 0) + add_taint_module(mod, TAINT_PROPRIETARY_MODULE); + +#ifdef CONFIG_MODVERSIONS + if ((mod->num_syms && !mod->crcs) + || (mod->num_gpl_syms && !mod->gpl_crcs) + || (mod->num_gpl_future_syms && !mod->gpl_future_crcs) +#ifdef CONFIG_UNUSED_SYMBOLS + || (mod->num_unused_syms && !mod->unused_crcs) + || (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs) +#endif + ) { + return try_to_force_load(mod, + "no versions for exported symbols"); + } +#endif + return 0; +} + +static void flush_module_icache(const struct module *mod) +{ + mm_segment_t old_fs; + + /* flush the icache in correct context */ + old_fs = get_fs(); + set_fs(KERNEL_DS); + + /* + * Flush the instruction cache, since we've played with text. + * Do it before processing of module parameters, so the module + * can provide parameter accessor functions of its own. + */ + if (mod->module_init) + flush_icache_range((unsigned long)mod->module_init, + (unsigned long)mod->module_init + + mod->init_size); + flush_icache_range((unsigned long)mod->module_core, + (unsigned long)mod->module_core + mod->core_size); + + set_fs(old_fs); +} + /* 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, @@ -2331,8 +2426,6 @@ static noinline struct module *load_module(void __user *umod, struct _ddebug *debug = NULL; unsigned int num_debug = 0; - mm_segment_t old_fs; - DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", umod, len, uargs); @@ -2453,20 +2546,13 @@ static noinline struct module *load_module(void __user *umod, if (err) goto free_init; - /* Set up license info based on the info section */ - set_license(mod, get_modinfo(sechdrs, infoindex, "license")); - - /* - * ndiswrapper is under GPL by itself, but loads proprietary modules. - * Don't use add_taint_module(), as it would prevent ndiswrapper from - * using GPL-only symbols it needs. - */ - if (strcmp(mod->name, "ndiswrapper") == 0) - add_taint(TAINT_PROPRIETARY_MODULE); + /* Now we've got everything in the final locations, we can + * find optional sections. */ + find_module_sections(mod, hdr, sechdrs, secstrings); - /* driverloader was caught wrongly pretending to be under GPL */ - if (strcmp(mod->name, "driverloader") == 0) - add_taint_module(mod, TAINT_PROPRIETARY_MODULE); + err = check_module_license_and_versions(mod, sechdrs); + if (err) + goto free_unload; /* Set up MODINFO_ATTR fields */ setup_modinfo(mod, sechdrs, infoindex); @@ -2477,47 +2563,9 @@ static noinline struct module *load_module(void __user *umod, if (err < 0) goto cleanup; - /* Now we've got everything in the final locations, we can - * find optional sections. */ - find_module_sections(mod, hdr, sechdrs, secstrings); - -#ifdef CONFIG_MODVERSIONS - if ((mod->num_syms && !mod->crcs) - || (mod->num_gpl_syms && !mod->gpl_crcs) - || (mod->num_gpl_future_syms && !mod->gpl_future_crcs) -#ifdef CONFIG_UNUSED_SYMBOLS - || (mod->num_unused_syms && !mod->unused_crcs) - || (mod->num_unused_gpl_syms && !mod->unused_gpl_crcs) -#endif - ) { - err = try_to_force_load(mod, - "no versions for exported symbols"); - if (err) - goto cleanup; - } -#endif - - /* Now do relocations. */ - for (i = 1; i < hdr->e_shnum; i++) { - const char *strtab = (char *)sechdrs[strindex].sh_addr; - unsigned int info = sechdrs[i].sh_info; - - /* Not a valid relocation section? */ - if (info >= hdr->e_shnum) - continue; - - /* Don't bother with non-allocated sections */ - if (!(sechdrs[info].sh_flags & SHF_ALLOC)) - continue; - - if (sechdrs[i].sh_type == SHT_REL) - err = apply_relocate(sechdrs, strtab, symindex, i,mod); - else if (sechdrs[i].sh_type == SHT_RELA) - err = apply_relocate_add(sechdrs, strtab, symindex, i, - mod); - if (err < 0) - goto cleanup; - } + err = apply_relocations(mod, hdr, sechdrs, symindex, strindex); + if (err < 0) + goto cleanup; /* Set up and sort exception table */ mod->extable = section_objs(hdr, sechdrs, secstrings, "__ex_table", @@ -2541,28 +2589,9 @@ static noinline struct module *load_module(void __user *umod, if (err < 0) goto cleanup; - /* flush the icache in correct context */ - old_fs = get_fs(); - set_fs(KERNEL_DS); - - /* - * Flush the instruction cache, since we've played with text. - * Do it before processing of module parameters, so the module - * can provide parameter accessor functions of its own. - */ - if (mod->module_init) - flush_icache_range((unsigned long)mod->module_init, - (unsigned long)mod->module_init - + mod->init_size); - flush_icache_range((unsigned long)mod->module_core, - (unsigned long)mod->module_core + mod->core_size); - - set_fs(old_fs); + flush_module_icache(mod); mod->args = args; - if (section_addr(hdr, sechdrs, secstrings, "__obsparm")) - printk(KERN_WARNING "%s: Ignoring obsolete parameters\n", - mod->name); /* Now sew it into the lists so we can get lockdep and oops * info during argument parsing. Noone should access us, since @@ -2619,6 +2648,7 @@ static noinline struct module *load_module(void __user *umod, module_arch_cleanup(mod); cleanup: free_modinfo(mod); + free_unload: module_unload_free(mod); free_init: module_free(mod, mod->module_init); -- cgit v0.10.2 From 44032e631691adf1f406843d5e54deb795973ff7 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 5 Aug 2010 12:59:05 -0600 Subject: module: reduce stack usage for each_symbol() And now that I'm looking at that call-chain (to see if it would make sense to use some other more specific lock - doesn't look like it: all the readers are using RCU and this is the only writer), I also give you this trivial one-liner. It changes each_symbol() to not put that constant array on the stack, resulting in changing movq $C.388.31095, %rsi #, tmp85 subq $376, %rsp #, movq %rdi, %rbx # fn, fn leaq -208(%rbp), %rdi #, tmp84 movq %rbx, %rdx # fn, rep movsl xorl %esi, %esi # leaq -208(%rbp), %rdi #, tmp87 movq %r12, %rcx # data, call each_symbol_in_section.clone.0 # into xorl %esi, %esi # subq $216, %rsp #, movq %rdi, %rbx # fn, fn movq $arr.31078, %rdi #, call each_symbol_in_section.clone.0 # which is not so much about being obviously shorter and simpler because we don't unnecessarily copy that constant array around onto the stack, but also about having a much smaller stack footprint (376 vs 216 bytes - see the update of 'rsp'). Signed-off-by: Linus Torvalds Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index b536db8..79545bd 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -227,7 +227,7 @@ bool each_symbol(bool (*fn)(const struct symsearch *arr, struct module *owner, unsigned int symnum, void *data), void *data) { struct module *mod; - const struct symsearch arr[] = { + static const struct symsearch arr[] = { { __start___ksymtab, __stop___ksymtab, __start___kcrctab, NOT_GPL_ONLY, false }, { __start___ksymtab_gpl, __stop___ksymtab_gpl, -- cgit v0.10.2 From 3264d3f9dd532ed9c3eb9491619e3f485b72747f Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Wed, 2 Jun 2010 11:01:06 -0700 Subject: module: add load_info Btw, here's a patch that _looks_ large, but it really pretty trivial, and sets things up so that it would be way easier to split off pieces of the module loading. The reason it looks large is that it creates a "module_info" structure that contains all the module state that we're building up while loading, instead of having individual variables for all the indices etc. So the patch ends up being large, because every "symindex" access instead becomes "info.index.sym" etc. That may be a few characters longer, but it then means that we can just pass a pointer to that "info" structure around. and let all the pieces fill it in very naturally. As an example of that, the patch also moves the initialization of all those convenience variables into a "setup_module_info()" function. And at this point it really does become very natural to start to peel off some of the error labels and move them into the helper functions - now the "truncated" case is gone, and is handled inside that setup function instead. So maybe you don't like this approach, and it does make the variable accesses a bit longer, but I don't think unreadably so. And the patch really does look big and scary, but there really should be absolutely no semantic changes - most of it was a trivial and mindless rename. In fact, it was so mindless that I on purpose kept the existing helper functions looking like this: - err = check_modinfo(mod, sechdrs, infoindex, versindex); + err = check_modinfo(mod, info.sechdrs, info.index.info, info.index.vers); rather than changing them to just take the "info" pointer. IOW, a second phase (if you think the approach is ok) would change that calling convention to just do err = check_modinfo(mod, &info); (and same for "layout_sections()", "layout_symtabs()" etc.) Similarly, while right now it makes things _look_ bigger, with things like this: versindex = find_sec(hdr, sechdrs, secstrings, "__versions"); becoming info->index.vers = find_sec(info->hdr, info->sechdrs, info->secstrings, "__versions"); in the new "setup_module_info()" function, that's again just a result of it being a search-and-replace patch. By using the 'info' pointer, we could just change the 'find_sec()' interface so that it ends up being info->index.vers = find_sec(info, "__versions"); instead, and then we'd actually have a shorter and more readable line. So for a lot of those mindless variable name expansions there's would be room for separate cleanups. I didn't move quite everything in there - if we do this to layout_symtabs, for example, we'd want to move the percpu, symoffs, stroffs, *strmap variables to be fields in that module_info structure too. But that's a much smaller patch, I moved just the really core stuff that is currently being set up and used in various parts. But even in this rough form, it removes close to 70 lines from that function (but adds 22 lines overall, of course - the structure definition, the helper function declarations and call-sites etc etc). Signed-off-by: Linus Torvalds Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 79545bd..cb40a4e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2148,8 +2148,17 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #endif -static int copy_and_check(Elf_Ehdr **hdrp, - const void __user *umod, unsigned long len) +struct load_info { + Elf_Ehdr *hdr; + unsigned long len; + Elf_Shdr *sechdrs; + char *secstrings, *args, *strtab; + struct { + unsigned int sym, str, mod, vers, info, pcpu; + } index; +}; + +static int copy_and_check(struct load_info *info, const void __user *umod, unsigned long len) { int err; Elf_Ehdr *hdr; @@ -2159,7 +2168,7 @@ static int copy_and_check(Elf_Ehdr **hdrp, /* Suck in entire file: we'll want most of it. */ /* vmalloc barfs on "unusual" numbers. Check here */ - if (len > 64 * 1024 * 1024 || (hdr = *hdrp = vmalloc(len)) == NULL) + if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL) return -ENOMEM; if (copy_from_user(hdr, umod, len) != 0) { @@ -2181,6 +2190,8 @@ static int copy_and_check(Elf_Ehdr **hdrp, err = -ENOEXEC; goto free_hdr; } + info->hdr = hdr; + info->len = len; return 0; free_hdr: @@ -2188,6 +2199,80 @@ free_hdr: return err; } +/* + * Set up our basic convenience variables (pointers to section headers, + * search for module section index etc), and do some basic section + * verification. + * + * Return the temporary module pointer (we'll replace it with the final + * one when we move the module sections around). + */ +static struct module *setup_load_info(struct load_info *info) +{ + unsigned int i; + struct module *mod; + + /* Set up the convenience variables */ + info->sechdrs = (void *)info->hdr + info->hdr->e_shoff; + info->secstrings = (void *)info->hdr + info->sechdrs[info->hdr->e_shstrndx].sh_offset; + info->sechdrs[0].sh_addr = 0; + + for (i = 1; i < info->hdr->e_shnum; i++) { + if (info->sechdrs[i].sh_type != SHT_NOBITS + && info->len < info->sechdrs[i].sh_offset + info->sechdrs[i].sh_size) + goto truncated; + + /* Mark all sections sh_addr with their address in the + temporary image. */ + info->sechdrs[i].sh_addr = (size_t)info->hdr + info->sechdrs[i].sh_offset; + + /* Internal symbols and strings. */ + if (info->sechdrs[i].sh_type == SHT_SYMTAB) { + info->index.sym = i; + info->index.str = info->sechdrs[i].sh_link; + info->strtab = (char *)info->hdr + info->sechdrs[info->index.str].sh_offset; + } +#ifndef CONFIG_MODULE_UNLOAD + /* Don't load .exit sections */ + if (strstarts(info->secstrings+info->sechdrs[i].sh_name, ".exit")) + info->sechdrs[i].sh_flags &= ~(unsigned long)SHF_ALLOC; +#endif + } + + info->index.mod = find_sec(info->hdr, info->sechdrs, info->secstrings, + ".gnu.linkonce.this_module"); + if (!info->index.mod) { + printk(KERN_WARNING "No module found in object\n"); + return ERR_PTR(-ENOEXEC); + } + /* This is temporary: point mod into copy of data. */ + mod = (void *)info->sechdrs[info->index.mod].sh_addr; + + if (info->index.sym == 0) { + printk(KERN_WARNING "%s: module has no symbols (stripped?)\n", + mod->name); + return ERR_PTR(-ENOEXEC); + } + + info->index.vers = find_sec(info->hdr, info->sechdrs, info->secstrings, "__versions"); + info->index.info = find_sec(info->hdr, info->sechdrs, info->secstrings, ".modinfo"); + info->index.pcpu = find_pcpusec(info->hdr, info->sechdrs, info->secstrings); + + /* Don't keep modinfo and version sections. */ + info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; + info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; + + /* Check module struct version now, before we try to use module. */ + if (!check_modstruct_version(info->sechdrs, info->index.vers, mod)) + return ERR_PTR(-ENOEXEC); + + return mod; + + truncated: + printk(KERN_ERR "Module len %lu truncated\n", info->len); + return ERR_PTR(-ENOEXEC); +} + static int check_modinfo(struct module *mod, const Elf_Shdr *sechdrs, unsigned int infoindex, unsigned int versindex) @@ -2412,13 +2497,7 @@ static noinline struct module *load_module(void __user *umod, unsigned long len, const char __user *uargs) { - Elf_Ehdr *hdr; - Elf_Shdr *sechdrs; - char *secstrings, *args, *strtab = NULL; - unsigned int i; - unsigned int symindex = 0; - unsigned int strindex = 0; - unsigned int modindex, versindex, infoindex, pcpuindex; + struct load_info info = { NULL, }; struct module *mod; long err; unsigned long symoffs, stroffs, *strmap; @@ -2429,80 +2508,28 @@ static noinline struct module *load_module(void __user *umod, DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", umod, len, uargs); - err = copy_and_check(&hdr, umod, len); + err = copy_and_check(&info, umod, len); if (err) return ERR_PTR(err); - /* Convenience variables */ - sechdrs = (void *)hdr + hdr->e_shoff; - secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; - sechdrs[0].sh_addr = 0; - - for (i = 1; i < hdr->e_shnum; i++) { - if (sechdrs[i].sh_type != SHT_NOBITS - && len < sechdrs[i].sh_offset + sechdrs[i].sh_size) - goto truncated; - - /* Mark all sections sh_addr with their address in the - temporary image. */ - sechdrs[i].sh_addr = (size_t)hdr + sechdrs[i].sh_offset; - - /* Internal symbols and strings. */ - if (sechdrs[i].sh_type == SHT_SYMTAB) { - symindex = i; - strindex = sechdrs[i].sh_link; - strtab = (char *)hdr + sechdrs[strindex].sh_offset; - } -#ifndef CONFIG_MODULE_UNLOAD - /* Don't load .exit sections */ - if (strstarts(secstrings+sechdrs[i].sh_name, ".exit")) - sechdrs[i].sh_flags &= ~(unsigned long)SHF_ALLOC; -#endif - } - - modindex = find_sec(hdr, sechdrs, secstrings, - ".gnu.linkonce.this_module"); - if (!modindex) { - printk(KERN_WARNING "No module found in object\n"); - err = -ENOEXEC; - goto free_hdr; - } - /* This is temporary: point mod into copy of data. */ - mod = (void *)sechdrs[modindex].sh_addr; - - if (symindex == 0) { - printk(KERN_WARNING "%s: module has no symbols (stripped?)\n", - mod->name); - err = -ENOEXEC; - goto free_hdr; - } - - versindex = find_sec(hdr, sechdrs, secstrings, "__versions"); - infoindex = find_sec(hdr, sechdrs, secstrings, ".modinfo"); - pcpuindex = find_pcpusec(hdr, sechdrs, secstrings); - - /* Don't keep modinfo and version sections. */ - sechdrs[infoindex].sh_flags &= ~(unsigned long)SHF_ALLOC; - sechdrs[versindex].sh_flags &= ~(unsigned long)SHF_ALLOC; - - /* Check module struct version now, before we try to use module. */ - if (!check_modstruct_version(sechdrs, versindex, mod)) { - err = -ENOEXEC; + mod = setup_load_info(&info); + if (IS_ERR(mod)) { + err = PTR_ERR(mod); goto free_hdr; } - err = check_modinfo(mod, sechdrs, infoindex, versindex); + err = check_modinfo(mod, info.sechdrs, info.index.info, info.index.vers); if (err) goto free_hdr; /* Now copy in args */ - args = strndup_user(uargs, ~0UL >> 1); - if (IS_ERR(args)) { - err = PTR_ERR(args); + info.args = strndup_user(uargs, ~0UL >> 1); + if (IS_ERR(info.args)) { + err = PTR_ERR(info.args); goto free_hdr; } - strmap = kzalloc(BITS_TO_LONGS(sechdrs[strindex].sh_size) + strmap = kzalloc(BITS_TO_LONGS(info.sechdrs[info.index.str].sh_size) * sizeof(long), GFP_KERNEL); if (!strmap) { err = -ENOMEM; @@ -2512,17 +2539,17 @@ static noinline struct module *load_module(void __user *umod, mod->state = MODULE_STATE_COMING; /* Allow arches to frob section contents and sizes. */ - err = module_frob_arch_sections(hdr, sechdrs, secstrings, mod); + err = module_frob_arch_sections(info.hdr, info.sechdrs, info.secstrings, mod); if (err < 0) goto free_mod; - if (pcpuindex) { + if (info.index.pcpu) { /* We have a special allocation for this section. */ - err = percpu_modalloc(mod, sechdrs[pcpuindex].sh_size, - sechdrs[pcpuindex].sh_addralign); + err = percpu_modalloc(mod, info.sechdrs[info.index.pcpu].sh_size, + info.sechdrs[info.index.pcpu].sh_addralign); if (err) goto free_mod; - sechdrs[pcpuindex].sh_flags &= ~(unsigned long)SHF_ALLOC; + info.sechdrs[info.index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; } /* Keep this around for failure path. */ percpu = mod_percpu(mod); @@ -2530,12 +2557,12 @@ static noinline struct module *load_module(void __user *umod, /* Determine total sizes, and put offsets in sh_entsize. For now this is done generically; there doesn't appear to be any special cases for the architectures. */ - layout_sections(mod, hdr, sechdrs, secstrings); - symoffs = layout_symtab(mod, sechdrs, symindex, strindex, hdr, - secstrings, &stroffs, strmap); + layout_sections(mod, info.hdr, info.sechdrs, info.secstrings); + symoffs = layout_symtab(mod, info.sechdrs, info.index.sym, info.index.str, info.hdr, + info.secstrings, &stroffs, strmap); /* Allocate and move to the final place */ - mod = move_module(mod, hdr, sechdrs, secstrings, modindex); + mod = move_module(mod, info.hdr, info.sechdrs, info.secstrings, info.index.mod); if (IS_ERR(mod)) { err = PTR_ERR(mod); goto free_percpu; @@ -2548,50 +2575,50 @@ static noinline struct module *load_module(void __user *umod, /* Now we've got everything in the final locations, we can * find optional sections. */ - find_module_sections(mod, hdr, sechdrs, secstrings); + find_module_sections(mod, info.hdr, info.sechdrs, info.secstrings); - err = check_module_license_and_versions(mod, sechdrs); + err = check_module_license_and_versions(mod, info.sechdrs); if (err) goto free_unload; /* Set up MODINFO_ATTR fields */ - setup_modinfo(mod, sechdrs, infoindex); + setup_modinfo(mod, info.sechdrs, info.index.info); /* Fix up syms, so that st_value is a pointer to location. */ - err = simplify_symbols(sechdrs, symindex, strtab, versindex, pcpuindex, + err = simplify_symbols(info.sechdrs, info.index.sym, info.strtab, info.index.vers, info.index.pcpu, mod); if (err < 0) goto cleanup; - err = apply_relocations(mod, hdr, sechdrs, symindex, strindex); + err = apply_relocations(mod, info.hdr, info.sechdrs, info.index.sym, info.index.str); if (err < 0) goto cleanup; /* Set up and sort exception table */ - mod->extable = section_objs(hdr, sechdrs, secstrings, "__ex_table", + mod->extable = section_objs(info.hdr, info.sechdrs, info.secstrings, "__ex_table", sizeof(*mod->extable), &mod->num_exentries); sort_extable(mod->extable, mod->extable + mod->num_exentries); /* Finally, copy percpu area over. */ - percpu_modcopy(mod, (void *)sechdrs[pcpuindex].sh_addr, - sechdrs[pcpuindex].sh_size); + percpu_modcopy(mod, (void *)info.sechdrs[info.index.pcpu].sh_addr, + info.sechdrs[info.index.pcpu].sh_size); - add_kallsyms(mod, sechdrs, hdr->e_shnum, symindex, strindex, - symoffs, stroffs, secstrings, strmap); + add_kallsyms(mod, info.sechdrs, info.hdr->e_shnum, info.index.sym, info.index.str, + symoffs, stroffs, info.secstrings, strmap); kfree(strmap); strmap = NULL; if (!mod->taints) - debug = section_objs(hdr, sechdrs, secstrings, "__verbose", + debug = section_objs(info.hdr, info.sechdrs, info.secstrings, "__verbose", sizeof(*debug), &num_debug); - err = module_finalize(hdr, sechdrs, mod); + err = module_finalize(info.hdr, info.sechdrs, mod); if (err < 0) goto cleanup; flush_module_icache(mod); - mod->args = args; + mod->args = info.args; /* Now sew it into the lists so we can get lockdep and oops * info during argument parsing. Noone should access us, since @@ -2625,11 +2652,11 @@ static noinline struct module *load_module(void __user *umod, if (err < 0) goto unlink; - add_sect_attrs(mod, hdr->e_shnum, secstrings, sechdrs); - add_notes_attrs(mod, hdr->e_shnum, secstrings, sechdrs); + add_sect_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); + add_notes_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); /* Get rid of temporary copy */ - vfree(hdr); + vfree(info.hdr); trace_module_load(mod); @@ -2657,16 +2684,11 @@ static noinline struct module *load_module(void __user *umod, free_percpu: free_percpu(percpu); free_mod: - kfree(args); + kfree(info.args); kfree(strmap); free_hdr: - vfree(hdr); + vfree(info.hdr); return ERR_PTR(err); - - truncated: - printk(KERN_ERR "Module len %lu truncated\n", len); - err = -ENOEXEC; - goto free_hdr; } /* Call module constructors. */ -- cgit v0.10.2 From 8b5f61a795fe37be090b0fd18b6b7271db9298e0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:06 -0600 Subject: module: refactor out section header rewriting Put all the "rewrite and check section headers" in one place. This adds another iteration over the sections, but it's far clearer. We iterate once for every find_section() so we already iterate over many times. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index cb40a4e..91f3ebe 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2158,6 +2158,7 @@ struct load_info { } index; }; +/* Sets info->hdr and info->len. */ static int copy_and_check(struct load_info *info, const void __user *umod, unsigned long len) { int err; @@ -2199,6 +2200,39 @@ free_hdr: return err; } +static int rewrite_section_headers(struct load_info *info) +{ + unsigned int i; + + /* This should always be true, but let's be sure. */ + info->sechdrs[0].sh_addr = 0; + + for (i = 1; i < info->hdr->e_shnum; i++) { + Elf_Shdr *shdr = &info->sechdrs[i]; + if (shdr->sh_type != SHT_NOBITS + && info->len < shdr->sh_offset + shdr->sh_size) { + printk(KERN_ERR "Module len %lu truncated\n", + info->len); + return -ENOEXEC; + } + + /* Mark all sections sh_addr with their address in the + temporary image. */ + shdr->sh_addr = (size_t)info->hdr + shdr->sh_offset; + +#ifndef CONFIG_MODULE_UNLOAD + /* Don't load .exit sections */ + if (strstarts(info->secstrings+shdr->sh_name, ".exit")) + shdr->sh_flags &= ~(unsigned long)SHF_ALLOC; +#endif + /* Don't keep modinfo and version sections. */ + if (!strcmp(info->secstrings+shdr->sh_name, "__versions") + || !strcmp(info->secstrings+shdr->sh_name, ".modinfo")) + shdr->sh_flags &= ~(unsigned long)SHF_ALLOC; + } + return 0; +} + /* * Set up our basic convenience variables (pointers to section headers, * search for module section index etc), and do some basic section @@ -2210,33 +2244,27 @@ free_hdr: static struct module *setup_load_info(struct load_info *info) { unsigned int i; + int err; struct module *mod; /* Set up the convenience variables */ info->sechdrs = (void *)info->hdr + info->hdr->e_shoff; - info->secstrings = (void *)info->hdr + info->sechdrs[info->hdr->e_shstrndx].sh_offset; - info->sechdrs[0].sh_addr = 0; - - for (i = 1; i < info->hdr->e_shnum; i++) { - if (info->sechdrs[i].sh_type != SHT_NOBITS - && info->len < info->sechdrs[i].sh_offset + info->sechdrs[i].sh_size) - goto truncated; + info->secstrings = (void *)info->hdr + + info->sechdrs[info->hdr->e_shstrndx].sh_offset; - /* Mark all sections sh_addr with their address in the - temporary image. */ - info->sechdrs[i].sh_addr = (size_t)info->hdr + info->sechdrs[i].sh_offset; + err = rewrite_section_headers(info); + if (err) + return ERR_PTR(err); - /* Internal symbols and strings. */ + /* Find internal symbols and strings. */ + for (i = 1; i < info->hdr->e_shnum; i++) { if (info->sechdrs[i].sh_type == SHT_SYMTAB) { info->index.sym = i; info->index.str = info->sechdrs[i].sh_link; - info->strtab = (char *)info->hdr + info->sechdrs[info->index.str].sh_offset; + info->strtab = (char *)info->hdr + + info->sechdrs[info->index.str].sh_offset; + break; } -#ifndef CONFIG_MODULE_UNLOAD - /* Don't load .exit sections */ - if (strstarts(info->secstrings+info->sechdrs[i].sh_name, ".exit")) - info->sechdrs[i].sh_flags &= ~(unsigned long)SHF_ALLOC; -#endif } info->index.mod = find_sec(info->hdr, info->sechdrs, info->secstrings, @@ -2258,19 +2286,11 @@ static struct module *setup_load_info(struct load_info *info) info->index.info = find_sec(info->hdr, info->sechdrs, info->secstrings, ".modinfo"); info->index.pcpu = find_pcpusec(info->hdr, info->sechdrs, info->secstrings); - /* Don't keep modinfo and version sections. */ - info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; - info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; - /* Check module struct version now, before we try to use module. */ if (!check_modstruct_version(info->sechdrs, info->index.vers, mod)) return ERR_PTR(-ENOEXEC); return mod; - - truncated: - printk(KERN_ERR "Module len %lu truncated\n", info->len); - return ERR_PTR(-ENOEXEC); } static int check_modinfo(struct module *mod, -- cgit v0.10.2 From d6df72a06e067139d491da4a9d14d92ad39e7a50 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:07 -0600 Subject: module: refactor out section header rewriting: FIX modversions We can't do the find_sec after removing the SHF_ALLOC flags; it won't find the sections. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 91f3ebe..ab07f67 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2225,11 +2225,13 @@ static int rewrite_section_headers(struct load_info *info) if (strstarts(info->secstrings+shdr->sh_name, ".exit")) shdr->sh_flags &= ~(unsigned long)SHF_ALLOC; #endif - /* Don't keep modinfo and version sections. */ - if (!strcmp(info->secstrings+shdr->sh_name, "__versions") - || !strcmp(info->secstrings+shdr->sh_name, ".modinfo")) - shdr->sh_flags &= ~(unsigned long)SHF_ALLOC; } + + /* Track but don't keep modinfo and version sections. */ + info->index.vers = find_sec(info->hdr, info->sechdrs, info->secstrings, "__versions"); + info->index.info = find_sec(info->hdr, info->sechdrs, info->secstrings, ".modinfo"); + info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; + info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; return 0; } @@ -2282,8 +2284,6 @@ static struct module *setup_load_info(struct load_info *info) return ERR_PTR(-ENOEXEC); } - info->index.vers = find_sec(info->hdr, info->sechdrs, info->secstrings, "__versions"); - info->index.info = find_sec(info->hdr, info->sechdrs, info->secstrings, ".modinfo"); info->index.pcpu = find_pcpusec(info->hdr, info->sechdrs, info->secstrings); /* Check module struct version now, before we try to use module. */ -- cgit v0.10.2 From eded41c1c6466081e0eb00d38719c6e6ee81a5d4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:07 -0600 Subject: module: kallsyms functions take struct load_info Simple refactor causes us to lift struct definition to top of file. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index ab07f67..79c4d6f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -110,6 +110,16 @@ int unregister_module_notifier(struct notifier_block * nb) } EXPORT_SYMBOL(unregister_module_notifier); +struct load_info { + Elf_Ehdr *hdr; + unsigned long len; + Elf_Shdr *sechdrs; + char *secstrings, *args, *strtab; + struct { + unsigned int sym, str, mod, vers, info, pcpu; + } index; +}; + /* We require a truly strong try_module_get(): 0 means failure due to ongoing or failed initialization etc. */ static inline int strong_try_module_get(struct module *mod) @@ -1909,11 +1919,10 @@ static int is_exported(const char *name, unsigned long value, } /* As per nm */ -static char elf_type(const Elf_Sym *sym, - Elf_Shdr *sechdrs, - const char *secstrings, - struct module *mod) +static char elf_type(const Elf_Sym *sym, const struct load_info *info) { + const Elf_Shdr *sechdrs = info->sechdrs; + if (ELF_ST_BIND(sym->st_info) == STB_WEAK) { if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT) return 'v'; @@ -1943,8 +1952,10 @@ static char elf_type(const Elf_Sym *sym, else return 'b'; } - if (strstarts(secstrings + sechdrs[sym->st_shndx].sh_name, ".debug")) + if (strstarts(info->secstrings + sechdrs[sym->st_shndx].sh_name, + ".debug")) { return 'n'; + } return '?'; } @@ -2021,35 +2032,30 @@ static unsigned long layout_symtab(struct module *mod, return symoffs; } -static void add_kallsyms(struct module *mod, - Elf_Shdr *sechdrs, - unsigned int shnum, - unsigned int symindex, - unsigned int strindex, +static void add_kallsyms(struct module *mod, struct load_info *info, unsigned long symoffs, unsigned long stroffs, - const char *secstrings, unsigned long *strmap) { unsigned int i, ndst; const Elf_Sym *src; Elf_Sym *dst; char *s; + Elf_Shdr *symsec = &info->sechdrs[info->index.sym]; - mod->symtab = (void *)sechdrs[symindex].sh_addr; - mod->num_symtab = sechdrs[symindex].sh_size / sizeof(Elf_Sym); - mod->strtab = (void *)sechdrs[strindex].sh_addr; + mod->symtab = (void *)symsec->sh_addr; + mod->num_symtab = symsec->sh_size / sizeof(Elf_Sym); + mod->strtab = info->strtab; /* Set types up while we still have access to sections. */ for (i = 0; i < mod->num_symtab; i++) - mod->symtab[i].st_info - = elf_type(&mod->symtab[i], sechdrs, secstrings, mod); + mod->symtab[i].st_info = elf_type(&mod->symtab[i], info); mod->core_symtab = dst = mod->module_core + symoffs; src = mod->symtab; *dst = *src; for (ndst = i = 1; i < mod->num_symtab; ++i, ++src) { - if (!is_core_symbol(src, sechdrs, shnum)) + if (!is_core_symbol(src, info->sechdrs, info->hdr->e_shnum)) continue; dst[ndst] = *src; dst[ndst].st_name = bitmap_weight(strmap, dst[ndst].st_name); @@ -2058,7 +2064,7 @@ static void add_kallsyms(struct module *mod, mod->core_num_syms = ndst; mod->core_strtab = s = mod->module_core + stroffs; - for (*s = 0, i = 1; i < sechdrs[strindex].sh_size; ++i) + for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i) if (test_bit(i, strmap)) *++s = mod->strtab[i]; } @@ -2075,15 +2081,10 @@ static inline unsigned long layout_symtab(struct module *mod, return 0; } -static inline void add_kallsyms(struct module *mod, - Elf_Shdr *sechdrs, - unsigned int shnum, - unsigned int symindex, - unsigned int strindex, - unsigned long symoffs, - unsigned long stroffs, - const char *secstrings, - const unsigned long *strmap) +static void add_kallsyms(struct module *mod, struct load_info *info, + unsigned long symoffs, + unsigned long stroffs, + unsigned long *strmap) { } #endif /* CONFIG_KALLSYMS */ @@ -2148,16 +2149,6 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #endif -struct load_info { - Elf_Ehdr *hdr; - unsigned long len; - Elf_Shdr *sechdrs; - char *secstrings, *args, *strtab; - struct { - unsigned int sym, str, mod, vers, info, pcpu; - } index; -}; - /* Sets info->hdr and info->len. */ static int copy_and_check(struct load_info *info, const void __user *umod, unsigned long len) { @@ -2623,8 +2614,7 @@ static noinline struct module *load_module(void __user *umod, percpu_modcopy(mod, (void *)info.sechdrs[info.index.pcpu].sh_addr, info.sechdrs[info.index.pcpu].sh_size); - add_kallsyms(mod, info.sechdrs, info.hdr->e_shnum, info.index.sym, info.index.str, - symoffs, stroffs, info.secstrings, strmap); + add_kallsyms(mod, &info, symoffs, stroffs, strmap); kfree(strmap); strmap = NULL; -- cgit v0.10.2 From 511ca6ae43fbe0a7c9e0b50ad275f7ef24ef3b58 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:08 -0600 Subject: module: fix crash in get_ksymbol() when oopsing in module init Andrew had the sole pleasure of tickling this bug in linux-next; when we set up "info->strtab" it's pointing into the temporary copy of the module. For most uses that is fine, but kallsyms keeps a pointer around during module load (inside mod->strtab). If we oops for some reason inside a module's init function, kallsyms will use the mod->strtab pointer into the now-freed temporary module copy. (Later oopses work fine: after init we overwrite mod->strtab to point to a compacted core-only strtab). Reported-by: Andrew "Grumpy" Morton Signed-off-by: Rusty "Buggy" Russell Tested-by: Andrew "Happy" Morton Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 79c4d6f..60cdd04 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2045,7 +2045,8 @@ static void add_kallsyms(struct module *mod, struct load_info *info, mod->symtab = (void *)symsec->sh_addr; mod->num_symtab = symsec->sh_size / sizeof(Elf_Sym); - mod->strtab = info->strtab; + /* Make sure we get permanent strtab: don't use info->strtab. */ + mod->strtab = (void *)info->sechdrs[info->index.str].sh_addr; /* Set types up while we still have access to sections. */ for (i = 0; i < mod->num_symtab; i++) -- cgit v0.10.2 From d913188c75191114051cf0bac75dad444c6080fa Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:08 -0600 Subject: module: layout_and_allocate layout_and_allocate() does everything up to and including the final struct module placement inside the allocated module memory. We have to store the symbol layout information in our struct load_info though. This avoids the nasty code we had before where 'mod' pointed first to the version inside the temporary allocation containing the entire file, then later was moved to point to the real struct module: now the main code only ever sees the final module address. (Includes fix for the Tony Luck-found Linus-diagnosed failure path error). Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 60cdd04..fb11e2a 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -115,6 +115,8 @@ struct load_info { unsigned long len; Elf_Shdr *sechdrs; char *secstrings, *args, *strtab; + unsigned long *strmap; + unsigned long symoffs, stroffs; struct { unsigned int sym, str, mod, vers, info, pcpu; } index; @@ -402,7 +404,8 @@ static int percpu_modalloc(struct module *mod, mod->percpu = __alloc_reserved_percpu(size, align); if (!mod->percpu) { printk(KERN_WARNING - "Could not allocate %lu bytes percpu data\n", size); + "%s: Could not allocate %lu bytes percpu data\n", + mod->name, size); return -ENOMEM; } mod->percpu_size = size; @@ -2032,10 +2035,7 @@ static unsigned long layout_symtab(struct module *mod, return symoffs; } -static void add_kallsyms(struct module *mod, struct load_info *info, - unsigned long symoffs, - unsigned long stroffs, - unsigned long *strmap) +static void add_kallsyms(struct module *mod, struct load_info *info) { unsigned int i, ndst; const Elf_Sym *src; @@ -2052,21 +2052,22 @@ static void add_kallsyms(struct module *mod, struct load_info *info, for (i = 0; i < mod->num_symtab; i++) mod->symtab[i].st_info = elf_type(&mod->symtab[i], info); - mod->core_symtab = dst = mod->module_core + symoffs; + mod->core_symtab = dst = mod->module_core + info->symoffs; src = mod->symtab; *dst = *src; for (ndst = i = 1; i < mod->num_symtab; ++i, ++src) { if (!is_core_symbol(src, info->sechdrs, info->hdr->e_shnum)) continue; dst[ndst] = *src; - dst[ndst].st_name = bitmap_weight(strmap, dst[ndst].st_name); + dst[ndst].st_name = bitmap_weight(info->strmap, + dst[ndst].st_name); ++ndst; } mod->core_num_syms = ndst; - mod->core_strtab = s = mod->module_core + stroffs; + mod->core_strtab = s = mod->module_core + info->stroffs; for (*s = 0, i = 1; i < info->sechdrs[info->index.str].sh_size; ++i) - if (test_bit(i, strmap)) + if (test_bit(i, info->strmap)) *++s = mod->strtab[i]; } #else @@ -2082,10 +2083,7 @@ static inline unsigned long layout_symtab(struct module *mod, return 0; } -static void add_kallsyms(struct module *mod, struct load_info *info, - unsigned long symoffs, - unsigned long stroffs, - unsigned long *strmap) +static void add_kallsyms(struct module *mod, struct load_info *info) { } #endif /* CONFIG_KALLSYMS */ @@ -2150,8 +2148,10 @@ static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, } #endif -/* Sets info->hdr and info->len. */ -static int copy_and_check(struct load_info *info, const void __user *umod, unsigned long len) +/* Sets info->hdr, info->len and info->args. */ +static int copy_and_check(struct load_info *info, + const void __user *umod, unsigned long len, + const char __user *uargs) { int err; Elf_Ehdr *hdr; @@ -2183,6 +2183,14 @@ static int copy_and_check(struct load_info *info, const void __user *umod, unsig err = -ENOEXEC; goto free_hdr; } + + /* Now copy in args */ + info->args = strndup_user(uargs, ~0UL >> 1); + if (IS_ERR(info->args)) { + err = PTR_ERR(info->args); + goto free_hdr; + } + info->hdr = hdr; info->len = len; return 0; @@ -2192,6 +2200,12 @@ free_hdr: return err; } +static void free_copy(struct load_info *info) +{ + kfree(info->args); + vfree(info->hdr); +} + static int rewrite_section_headers(struct load_info *info) { unsigned int i; @@ -2385,9 +2399,9 @@ static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, mod->name); } -static struct module *move_module(struct module *mod, - Elf_Ehdr *hdr, Elf_Shdr *sechdrs, - const char *secstrings, unsigned modindex) +static int move_module(struct module *mod, + Elf_Ehdr *hdr, Elf_Shdr *sechdrs, + const char *secstrings, unsigned modindex) { int i; void *ptr; @@ -2401,7 +2415,7 @@ static struct module *move_module(struct module *mod, */ kmemleak_not_leak(ptr); if (!ptr) - return ERR_PTR(-ENOMEM); + return -ENOMEM; memset(ptr, 0, mod->core_size); mod->module_core = ptr; @@ -2416,7 +2430,7 @@ static struct module *move_module(struct module *mod, kmemleak_ignore(ptr); if (!ptr && mod->init_size) { module_free(mod, mod->module_core); - return ERR_PTR(-ENOMEM); + return -ENOMEM; } memset(ptr, 0, mod->init_size); mod->module_init = ptr; @@ -2443,10 +2457,8 @@ static struct module *move_module(struct module *mod, 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; + + return 0; } static int check_module_license_and_versions(struct module *mod, @@ -2503,87 +2515,107 @@ static void flush_module_icache(const struct module *mod) set_fs(old_fs); } -/* 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, - unsigned long len, - const char __user *uargs) +static struct module *layout_and_allocate(struct load_info *info) { - struct load_info info = { NULL, }; + /* Module within temporary copy. */ struct module *mod; - long err; - unsigned long symoffs, stroffs, *strmap; - void __percpu *percpu; - struct _ddebug *debug = NULL; - unsigned int num_debug = 0; + int err; - DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", - umod, len, uargs); + mod = setup_load_info(info); + if (IS_ERR(mod)) + return mod; - err = copy_and_check(&info, umod, len); + err = check_modinfo(mod, info->sechdrs, info->index.info, info->index.vers); if (err) return ERR_PTR(err); - mod = setup_load_info(&info); - if (IS_ERR(mod)) { - err = PTR_ERR(mod); - goto free_hdr; - } - - err = check_modinfo(mod, info.sechdrs, info.index.info, info.index.vers); - if (err) - goto free_hdr; - - /* Now copy in args */ - info.args = strndup_user(uargs, ~0UL >> 1); - if (IS_ERR(info.args)) { - err = PTR_ERR(info.args); - goto free_hdr; - } - - strmap = kzalloc(BITS_TO_LONGS(info.sechdrs[info.index.str].sh_size) - * sizeof(long), GFP_KERNEL); - if (!strmap) { - err = -ENOMEM; - goto free_mod; - } - - mod->state = MODULE_STATE_COMING; - /* Allow arches to frob section contents and sizes. */ - err = module_frob_arch_sections(info.hdr, info.sechdrs, info.secstrings, mod); + err = module_frob_arch_sections(info->hdr, info->sechdrs, info->secstrings, mod); if (err < 0) - goto free_mod; + goto free_args; - if (info.index.pcpu) { + if (info->index.pcpu) { /* We have a special allocation for this section. */ - err = percpu_modalloc(mod, info.sechdrs[info.index.pcpu].sh_size, - info.sechdrs[info.index.pcpu].sh_addralign); + err = percpu_modalloc(mod, info->sechdrs[info->index.pcpu].sh_size, + info->sechdrs[info->index.pcpu].sh_addralign); if (err) - goto free_mod; - info.sechdrs[info.index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; + goto free_args; + info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; } - /* Keep this around for failure path. */ - percpu = mod_percpu(mod); /* Determine total sizes, and put offsets in sh_entsize. For now this is done generically; there doesn't appear to be any special cases for the architectures. */ - layout_sections(mod, info.hdr, info.sechdrs, info.secstrings); - symoffs = layout_symtab(mod, info.sechdrs, info.index.sym, info.index.str, info.hdr, - info.secstrings, &stroffs, strmap); + layout_sections(mod, info->hdr, info->sechdrs, info->secstrings); + + info->strmap = kzalloc(BITS_TO_LONGS(info->sechdrs[info->index.str].sh_size) + * sizeof(long), GFP_KERNEL); + if (!info->strmap) { + err = -ENOMEM; + goto free_percpu; + } + info->symoffs = layout_symtab(mod, info->sechdrs, info->index.sym, info->index.str, info->hdr, + info->secstrings, &info->stroffs, info->strmap); /* Allocate and move to the final place */ - mod = move_module(mod, info.hdr, info.sechdrs, info.secstrings, info.index.mod); + err = move_module(mod, info->hdr, info->sechdrs, info->secstrings, info->index.mod); + if (err) + goto free_strmap; + + /* Module has been copied to its final place now: return it. */ + mod = (void *)info->sechdrs[info->index.mod].sh_addr; + kmemleak_load_module(mod, info->hdr, info->sechdrs, info->secstrings); + return mod; + +free_strmap: + kfree(info->strmap); +free_percpu: + percpu_modfree(mod); +free_args: + kfree(info->args); + return ERR_PTR(err); +} + +/* mod is no longer valid after this! */ +static void module_deallocate(struct module *mod, struct load_info *info) +{ + kfree(info->strmap); + percpu_modfree(mod); + module_free(mod, mod->module_init); + module_free(mod, mod->module_core); +} + +/* 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, + unsigned long len, + const char __user *uargs) +{ + struct load_info info = { NULL, }; + struct module *mod; + long err; + struct _ddebug *debug = NULL; + unsigned int num_debug = 0; + + DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", + umod, len, uargs); + + /* Copy in the blobs from userspace, check they are vaguely sane. */ + err = copy_and_check(&info, umod, len, uargs); + if (err) + return ERR_PTR(err); + + /* Figure out module layout, and allocate all the memory. */ + mod = layout_and_allocate(&info); if (IS_ERR(mod)) { err = PTR_ERR(mod); - goto free_percpu; + goto free_copy; } /* Now we've moved module, initialize linked lists, etc. */ err = module_unload_init(mod); if (err) - goto free_init; + goto free_module; /* Now we've got everything in the final locations, we can * find optional sections. */ @@ -2600,11 +2632,11 @@ static noinline struct module *load_module(void __user *umod, err = simplify_symbols(info.sechdrs, info.index.sym, info.strtab, info.index.vers, info.index.pcpu, mod); if (err < 0) - goto cleanup; + goto free_modinfo; err = apply_relocations(mod, info.hdr, info.sechdrs, info.index.sym, info.index.str); if (err < 0) - goto cleanup; + goto free_modinfo; /* Set up and sort exception table */ mod->extable = section_objs(info.hdr, info.sechdrs, info.secstrings, "__ex_table", @@ -2615,9 +2647,7 @@ static noinline struct module *load_module(void __user *umod, percpu_modcopy(mod, (void *)info.sechdrs[info.index.pcpu].sh_addr, info.sechdrs[info.index.pcpu].sh_size); - add_kallsyms(mod, &info, symoffs, stroffs, strmap); - kfree(strmap); - strmap = NULL; + add_kallsyms(mod, &info); if (!mod->taints) debug = section_objs(info.hdr, info.sechdrs, info.secstrings, "__verbose", @@ -2625,12 +2655,14 @@ static noinline struct module *load_module(void __user *umod, err = module_finalize(info.hdr, info.sechdrs, mod); if (err < 0) - goto cleanup; + goto free_modinfo; flush_module_icache(mod); mod->args = info.args; + mod->state = MODULE_STATE_COMING; + /* Now sew it into the lists so we can get lockdep and oops * info during argument parsing. Noone should access us, since * strong_try_module_get() will fail. @@ -2666,8 +2698,9 @@ static noinline struct module *load_module(void __user *umod, add_sect_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); add_notes_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); - /* Get rid of temporary copy */ - vfree(info.hdr); + /* Get rid of temporary copy and strmap. */ + kfree(info.strmap); + free_copy(&info); trace_module_load(mod); @@ -2684,21 +2717,14 @@ static noinline struct module *load_module(void __user *umod, mutex_unlock(&module_mutex); synchronize_sched(); module_arch_cleanup(mod); - cleanup: + free_modinfo: free_modinfo(mod); free_unload: module_unload_free(mod); - free_init: - module_free(mod, mod->module_init); - module_free(mod, mod->module_core); - /* mod will be freed with core. Don't access it beyond this line! */ - free_percpu: - free_percpu(percpu); - free_mod: - kfree(info.args); - kfree(strmap); - free_hdr: - vfree(info.hdr); + free_module: + module_deallocate(mod, &info); + free_copy: + free_copy(&info); return ERR_PTR(err); } -- cgit v0.10.2 From 8f6d037815466cb25e7de8f00536eca71d94d4c3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:09 -0600 Subject: module: sysfs cleanup We change the sysfs functions to take struct load_info, and call them all in mod_sysfs_setup(). We also clean up the #ifdefs a little. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index fb11e2a..fd8d46c 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1126,8 +1126,9 @@ static const struct kernel_symbol *resolve_symbol_wait(Elf_Shdr *sechdrs, * /sys/module/foo/sections stuff * J. Corbet */ -#if defined(CONFIG_KALLSYMS) && defined(CONFIG_SYSFS) +#ifdef CONFIG_SYSFS +#ifdef CONFIG_KALLSYMS static inline bool sect_empty(const Elf_Shdr *sect) { return !(sect->sh_flags & SHF_ALLOC) || sect->sh_size == 0; @@ -1164,8 +1165,7 @@ static void free_sect_attrs(struct module_sect_attrs *sect_attrs) kfree(sect_attrs); } -static void add_sect_attrs(struct module *mod, unsigned int nsect, - char *secstrings, Elf_Shdr *sechdrs) +static void add_sect_attrs(struct module *mod, const struct load_info *info) { unsigned int nloaded = 0, i, size[2]; struct module_sect_attrs *sect_attrs; @@ -1173,8 +1173,8 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect, struct attribute **gattr; /* Count loaded sections and allocate structures */ - for (i = 0; i < nsect; i++) - if (!sect_empty(&sechdrs[i])) + for (i = 0; i < info->hdr->e_shnum; i++) + if (!sect_empty(&info->sechdrs[i])) nloaded++; size[0] = ALIGN(sizeof(*sect_attrs) + nloaded * sizeof(sect_attrs->attrs[0]), @@ -1191,11 +1191,12 @@ static void add_sect_attrs(struct module *mod, unsigned int nsect, sect_attrs->nsections = 0; sattr = §_attrs->attrs[0]; gattr = §_attrs->grp.attrs[0]; - for (i = 0; i < nsect; i++) { - if (sect_empty(&sechdrs[i])) + for (i = 0; i < info->hdr->e_shnum; i++) { + Elf_Shdr *sec = &info->sechdrs[i]; + if (sect_empty(sec)) continue; - sattr->address = sechdrs[i].sh_addr; - sattr->name = kstrdup(secstrings + sechdrs[i].sh_name, + sattr->address = sec->sh_addr; + sattr->name = kstrdup(info->secstrings + sec->sh_name, GFP_KERNEL); if (sattr->name == NULL) goto out; @@ -1263,8 +1264,7 @@ static void free_notes_attrs(struct module_notes_attrs *notes_attrs, kfree(notes_attrs); } -static void add_notes_attrs(struct module *mod, unsigned int nsect, - char *secstrings, Elf_Shdr *sechdrs) +static void add_notes_attrs(struct module *mod, const struct load_info *info) { unsigned int notes, loaded, i; struct module_notes_attrs *notes_attrs; @@ -1276,9 +1276,9 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect, /* Count notes sections and allocate structures. */ notes = 0; - for (i = 0; i < nsect; i++) - if (!sect_empty(&sechdrs[i]) && - (sechdrs[i].sh_type == SHT_NOTE)) + for (i = 0; i < info->hdr->e_shnum; i++) + if (!sect_empty(&info->sechdrs[i]) && + (info->sechdrs[i].sh_type == SHT_NOTE)) ++notes; if (notes == 0) @@ -1292,15 +1292,15 @@ static void add_notes_attrs(struct module *mod, unsigned int nsect, notes_attrs->notes = notes; nattr = ¬es_attrs->attrs[0]; - for (loaded = i = 0; i < nsect; ++i) { - if (sect_empty(&sechdrs[i])) + for (loaded = i = 0; i < info->hdr->e_shnum; ++i) { + if (sect_empty(&info->sechdrs[i])) continue; - if (sechdrs[i].sh_type == SHT_NOTE) { + if (info->sechdrs[i].sh_type == SHT_NOTE) { sysfs_bin_attr_init(nattr); nattr->attr.name = mod->sect_attrs->attrs[loaded].name; nattr->attr.mode = S_IRUGO; - nattr->size = sechdrs[i].sh_size; - nattr->private = (void *) sechdrs[i].sh_addr; + nattr->size = info->sechdrs[i].sh_size; + nattr->private = (void *) info->sechdrs[i].sh_addr; nattr->read = module_notes_read; ++nattr; } @@ -1331,8 +1331,8 @@ static void remove_notes_attrs(struct module *mod) #else -static inline void add_sect_attrs(struct module *mod, unsigned int nsect, - char *sectstrings, Elf_Shdr *sechdrs) +static inline void add_sect_attrs(struct module *mod, + const struct load_info *info) { } @@ -1340,17 +1340,16 @@ static inline void remove_sect_attrs(struct module *mod) { } -static inline void add_notes_attrs(struct module *mod, unsigned int nsect, - char *sectstrings, Elf_Shdr *sechdrs) +static inline void add_notes_attrs(struct module *mod, + const struct load_info *info) { } static inline void remove_notes_attrs(struct module *mod) { } -#endif +#endif /* CONFIG_KALLSYMS */ -#ifdef CONFIG_SYSFS static void add_usage_links(struct module *mod) { #ifdef CONFIG_MODULE_UNLOAD @@ -1455,6 +1454,7 @@ out: } static int mod_sysfs_setup(struct module *mod, + const struct load_info *info, struct kernel_param *kparam, unsigned int num_params) { @@ -1479,6 +1479,8 @@ static int mod_sysfs_setup(struct module *mod, goto out_unreg_param; add_usage_links(mod); + add_sect_attrs(mod, info); + add_notes_attrs(mod, info); kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD); return 0; @@ -1495,32 +1497,26 @@ out: static void mod_sysfs_fini(struct module *mod) { + remove_notes_attrs(mod); + remove_sect_attrs(mod); kobject_put(&mod->mkobj.kobj); } -#else /* CONFIG_SYSFS */ +#else /* !CONFIG_SYSFS */ -static inline int mod_sysfs_init(struct module *mod) +static int mod_sysfs_init(struct module *mod) { return 0; } -static inline int mod_sysfs_setup(struct module *mod, +static int mod_sysfs_setup(struct module *mod, + const struct load_info *info, struct kernel_param *kparam, unsigned int num_params) { return 0; } -static inline int module_add_modinfo_attrs(struct module *mod) -{ - return 0; -} - -static inline void module_remove_modinfo_attrs(struct module *mod) -{ -} - static void mod_sysfs_fini(struct module *mod) { } @@ -1561,8 +1557,6 @@ static void free_module(struct module *mod) mutex_lock(&module_mutex); stop_machine(__unlink_module, mod, NULL); mutex_unlock(&module_mutex); - remove_notes_attrs(mod); - remove_sect_attrs(mod); mod_kobject_remove(mod); /* Remove dynamic debug info */ @@ -2691,13 +2685,10 @@ static noinline struct module *load_module(void __user *umod, if (err < 0) goto unlink; - err = mod_sysfs_setup(mod, mod->kp, mod->num_kp); + err = mod_sysfs_setup(mod, &info, mod->kp, mod->num_kp); if (err < 0) goto unlink; - add_sect_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); - add_notes_attrs(mod, info.hdr->e_shnum, info.secstrings, info.sechdrs); - /* Get rid of temporary copy and strmap. */ kfree(info.strmap); free_copy(&info); -- cgit v0.10.2 From 36b0360d17dc3928cc96347a18a3a1cdbb7e506d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:09 -0600 Subject: module: fix sysfs cleanup for !CONFIG_SYSFS Restore the stub module_remove_modinfo_attrs, remove the now-unused !CONFIG_SYSFS module_sysfs_init. Also, rename mod_kobject_remove() to mod_sysfs_teardown() as it is the logical counterpart to mod_sysfs_setup now. Reported-by: Randy Dunlap Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index fd8d46c..a64b26c 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1504,11 +1504,6 @@ static void mod_sysfs_fini(struct module *mod) #else /* !CONFIG_SYSFS */ -static int mod_sysfs_init(struct module *mod) -{ - return 0; -} - static int mod_sysfs_setup(struct module *mod, const struct load_info *info, struct kernel_param *kparam, @@ -1521,13 +1516,17 @@ static void mod_sysfs_fini(struct module *mod) { } +static void module_remove_modinfo_attrs(struct module *mod) +{ +} + static void del_usage_links(struct module *mod) { } #endif /* CONFIG_SYSFS */ -static void mod_kobject_remove(struct module *mod) +static void mod_sysfs_teardown(struct module *mod) { del_usage_links(mod); module_remove_modinfo_attrs(mod); @@ -1557,7 +1556,7 @@ static void free_module(struct module *mod) mutex_lock(&module_mutex); stop_machine(__unlink_module, mod, NULL); mutex_unlock(&module_mutex); - mod_kobject_remove(mod); + mod_sysfs_teardown(mod); /* Remove dynamic debug info */ ddebug_remove_module(mod->name); -- cgit v0.10.2 From 49668688dd5a5f46c72f965835388ed16c596055 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:10 -0600 Subject: module: pass load_info into other functions Pass the struct load_info into all the other functions in module loading. This neatens things and makes them more consistent. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index a64b26c..29dd232 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -152,42 +152,38 @@ void __module_put_and_exit(struct module *mod, long code) EXPORT_SYMBOL(__module_put_and_exit); /* Find a module section: 0 means not found. */ -static unsigned int find_sec(Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings, - const char *name) +static unsigned int find_sec(const struct load_info *info, const char *name) { unsigned int i; - for (i = 1; i < hdr->e_shnum; i++) + for (i = 1; i < info->hdr->e_shnum; i++) { + Elf_Shdr *shdr = &info->sechdrs[i]; /* Alloc bit cleared means "ignore it." */ - if ((sechdrs[i].sh_flags & SHF_ALLOC) - && strcmp(secstrings+sechdrs[i].sh_name, name) == 0) + if ((shdr->sh_flags & SHF_ALLOC) + && strcmp(info->secstrings + shdr->sh_name, name) == 0) return i; + } return 0; } /* Find a module section, or NULL. */ -static void *section_addr(Elf_Ehdr *hdr, Elf_Shdr *shdrs, - const char *secstrings, const char *name) +static void *section_addr(const struct load_info *info, const char *name) { /* Section 0 has sh_addr 0. */ - return (void *)shdrs[find_sec(hdr, shdrs, secstrings, name)].sh_addr; + return (void *)info->sechdrs[find_sec(info, name)].sh_addr; } /* Find a module section, or NULL. Fill in number of "objects" in section. */ -static void *section_objs(Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings, +static void *section_objs(const struct load_info *info, const char *name, size_t object_size, unsigned int *num) { - unsigned int sec = find_sec(hdr, sechdrs, secstrings, name); + unsigned int sec = find_sec(info, name); /* Section 0 has sh_addr 0 and sh_size 0. */ - *num = sechdrs[sec].sh_size / object_size; - return (void *)sechdrs[sec].sh_addr; + *num = info->sechdrs[sec].sh_size / object_size; + return (void *)info->sechdrs[sec].sh_addr; } /* Provided by the linker */ @@ -417,11 +413,9 @@ static void percpu_modfree(struct module *mod) free_percpu(mod->percpu); } -static unsigned int find_pcpusec(Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings) +static unsigned int find_pcpusec(struct load_info *info) { - return find_sec(hdr, sechdrs, secstrings, ".data..percpu"); + return find_sec(info, ".data..percpu"); } static void percpu_modcopy(struct module *mod, @@ -481,9 +475,7 @@ static inline int percpu_modalloc(struct module *mod, static inline void percpu_modfree(struct module *mod) { } -static inline unsigned int find_pcpusec(Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings) +static unsigned int find_pcpusec(struct load_info *info) { return 0; } @@ -1067,10 +1059,9 @@ static inline int same_magic(const char *amagic, const char *bmagic, #endif /* CONFIG_MODVERSIONS */ /* Resolve a symbol for this module. I.e. if we find one, record usage. */ -static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs, - unsigned int versindex, +static const struct kernel_symbol *resolve_symbol(struct module *mod, + const struct load_info *info, const char *name, - struct module *mod, char ownername[]) { struct module *owner; @@ -1084,7 +1075,8 @@ static const struct kernel_symbol *resolve_symbol(Elf_Shdr *sechdrs, if (!sym) goto unlock; - if (!check_version(sechdrs, versindex, name, mod, crc, owner)) { + if (!check_version(info->sechdrs, info->index.vers, name, mod, crc, + owner)) { sym = ERR_PTR(-EINVAL); goto getname; } @@ -1103,21 +1095,20 @@ unlock: return sym; } -static const struct kernel_symbol *resolve_symbol_wait(Elf_Shdr *sechdrs, - unsigned int versindex, - const char *name, - struct module *mod) +static const struct kernel_symbol * +resolve_symbol_wait(struct module *mod, + const struct load_info *info, + const char *name) { const struct kernel_symbol *ksym; - char ownername[MODULE_NAME_LEN]; + char owner[MODULE_NAME_LEN]; if (wait_event_interruptible_timeout(module_wq, - !IS_ERR(ksym = resolve_symbol(sechdrs, versindex, name, - mod, ownername)) || - PTR_ERR(ksym) != -EBUSY, + !IS_ERR(ksym = resolve_symbol(mod, info, name, owner)) + || PTR_ERR(ksym) != -EBUSY, 30 * HZ) <= 0) { printk(KERN_WARNING "%s: gave up waiting for init of module %s.\n", - mod->name, ownername); + mod->name, owner); } return ksym; } @@ -1640,25 +1631,23 @@ static int verify_export_symbols(struct module *mod) } /* Change all symbols so that st_value encodes the pointer directly. */ -static int simplify_symbols(Elf_Shdr *sechdrs, - unsigned int symindex, - const char *strtab, - unsigned int versindex, - unsigned int pcpuindex, - struct module *mod) -{ - Elf_Sym *sym = (void *)sechdrs[symindex].sh_addr; +static int simplify_symbols(struct module *mod, const struct load_info *info) +{ + Elf_Shdr *symsec = &info->sechdrs[info->index.sym]; + Elf_Sym *sym = (void *)symsec->sh_addr; unsigned long secbase; - unsigned int i, n = sechdrs[symindex].sh_size / sizeof(Elf_Sym); + unsigned int i; int ret = 0; const struct kernel_symbol *ksym; - for (i = 1; i < n; i++) { + for (i = 1; i < symsec->sh_size / sizeof(Elf_Sym); i++) { + const char *name = info->strtab + sym[i].st_name; + switch (sym[i].st_shndx) { case SHN_COMMON: /* We compiled with -fno-common. These are not supposed to happen. */ - DEBUGP("Common symbol: %s\n", strtab + sym[i].st_name); + DEBUGP("Common symbol: %s\n", name); printk("%s: please compile with -fno-common\n", mod->name); ret = -ENOEXEC; @@ -1671,9 +1660,7 @@ static int simplify_symbols(Elf_Shdr *sechdrs, break; case SHN_UNDEF: - ksym = resolve_symbol_wait(sechdrs, versindex, - strtab + sym[i].st_name, - mod); + ksym = resolve_symbol_wait(mod, info, name); /* Ok if resolved. */ if (ksym && !IS_ERR(ksym)) { sym[i].st_value = ksym->value; @@ -1685,17 +1672,16 @@ static int simplify_symbols(Elf_Shdr *sechdrs, break; printk(KERN_WARNING "%s: Unknown symbol %s (err %li)\n", - mod->name, strtab + sym[i].st_name, - PTR_ERR(ksym)); + mod->name, name, PTR_ERR(ksym)); ret = PTR_ERR(ksym) ?: -ENOENT; break; default: /* Divert to percpu allocation if a percpu var. */ - if (sym[i].st_shndx == pcpuindex) + if (sym[i].st_shndx == info->index.pcpu) secbase = (unsigned long)mod_percpu(mod); else - secbase = sechdrs[sym[i].st_shndx].sh_addr; + secbase = info->sechdrs[sym[i].st_shndx].sh_addr; sym[i].st_value += secbase; break; } @@ -1704,33 +1690,29 @@ static int simplify_symbols(Elf_Shdr *sechdrs, return ret; } -static int apply_relocations(struct module *mod, - Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - unsigned int symindex, - unsigned int strindex) +static int apply_relocations(struct module *mod, const struct load_info *info) { unsigned int i; int err = 0; /* Now do relocations. */ - for (i = 1; i < hdr->e_shnum; i++) { - const char *strtab = (char *)sechdrs[strindex].sh_addr; - unsigned int info = sechdrs[i].sh_info; + for (i = 1; i < info->hdr->e_shnum; i++) { + unsigned int infosec = info->sechdrs[i].sh_info; /* Not a valid relocation section? */ - if (info >= hdr->e_shnum) + if (infosec >= info->hdr->e_shnum) continue; /* Don't bother with non-allocated sections */ - if (!(sechdrs[info].sh_flags & SHF_ALLOC)) + if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC)) continue; - if (sechdrs[i].sh_type == SHT_REL) - err = apply_relocate(sechdrs, strtab, symindex, i, mod); - else if (sechdrs[i].sh_type == SHT_RELA) - err = apply_relocate_add(sechdrs, strtab, symindex, i, - mod); + if (info->sechdrs[i].sh_type == SHT_REL) + err = apply_relocate(info->sechdrs, info->strtab, + info->index.sym, i, mod); + else if (info->sechdrs[i].sh_type == SHT_RELA) + err = apply_relocate_add(info->sechdrs, info->strtab, + info->index.sym, i, mod); if (err < 0) break; } @@ -1761,10 +1743,7 @@ static long get_offset(struct module *mod, unsigned int *size, might -- code, read-only data, read-write data, small data. Tally sizes, and place the offsets into sh_entsize fields: high bit means it belongs in init. */ -static void layout_sections(struct module *mod, - const Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings) +static void layout_sections(struct module *mod, struct load_info *info) { static unsigned long const masks[][2] = { /* NOTE: all executable code must be the first section @@ -1777,21 +1756,22 @@ static void layout_sections(struct module *mod, }; unsigned int m, i; - for (i = 0; i < hdr->e_shnum; i++) - sechdrs[i].sh_entsize = ~0UL; + for (i = 0; i < info->hdr->e_shnum; i++) + info->sechdrs[i].sh_entsize = ~0UL; DEBUGP("Core section allocation order:\n"); for (m = 0; m < ARRAY_SIZE(masks); ++m) { - for (i = 0; i < hdr->e_shnum; ++i) { - Elf_Shdr *s = &sechdrs[i]; + for (i = 0; i < info->hdr->e_shnum; ++i) { + Elf_Shdr *s = &info->sechdrs[i]; + const char *sname = info->secstrings + s->sh_name; if ((s->sh_flags & masks[m][0]) != masks[m][0] || (s->sh_flags & masks[m][1]) || s->sh_entsize != ~0UL - || strstarts(secstrings + s->sh_name, ".init")) + || strstarts(sname, ".init")) continue; s->sh_entsize = get_offset(mod, &mod->core_size, s, i); - DEBUGP("\t%s\n", secstrings + s->sh_name); + DEBUGP("\t%s\n", name); } if (m == 0) mod->core_text_size = mod->core_size; @@ -1799,17 +1779,18 @@ static void layout_sections(struct module *mod, DEBUGP("Init section allocation order:\n"); for (m = 0; m < ARRAY_SIZE(masks); ++m) { - for (i = 0; i < hdr->e_shnum; ++i) { - Elf_Shdr *s = &sechdrs[i]; + for (i = 0; i < info->hdr->e_shnum; ++i) { + Elf_Shdr *s = &info->sechdrs[i]; + const char *sname = info->secstrings + s->sh_name; if ((s->sh_flags & masks[m][0]) != masks[m][0] || (s->sh_flags & masks[m][1]) || s->sh_entsize != ~0UL - || !strstarts(secstrings + s->sh_name, ".init")) + || !strstarts(sname, ".init")) continue; s->sh_entsize = (get_offset(mod, &mod->init_size, s, i) | INIT_OFFSET_MASK); - DEBUGP("\t%s\n", secstrings + s->sh_name); + DEBUGP("\t%s\n", sname); } if (m == 0) mod->init_text_size = mod->init_size; @@ -1848,33 +1829,28 @@ static char *next_string(char *string, unsigned long *secsize) return string; } -static char *get_modinfo(const Elf_Shdr *sechdrs, - unsigned int info, - const char *tag) +static char *get_modinfo(struct load_info *info, const char *tag) { char *p; unsigned int taglen = strlen(tag); - unsigned long size = sechdrs[info].sh_size; + Elf_Shdr *infosec = &info->sechdrs[info->index.info]; + unsigned long size = infosec->sh_size; - for (p = (char *)sechdrs[info].sh_addr; p; p = next_string(p, &size)) { + for (p = (char *)infosec->sh_addr; p; p = next_string(p, &size)) { if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=') return p + taglen + 1; } return NULL; } -static void setup_modinfo(struct module *mod, Elf_Shdr *sechdrs, - unsigned int infoindex) +static void setup_modinfo(struct module *mod, struct load_info *info) { struct module_attribute *attr; int i; for (i = 0; (attr = modinfo_attrs[i]); i++) { if (attr->setup) - attr->setup(mod, - get_modinfo(sechdrs, - infoindex, - attr->attr.name)); + attr->setup(mod, get_modinfo(info, attr->attr.name)); } } @@ -1976,56 +1952,45 @@ static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs, return true; } -static unsigned long layout_symtab(struct module *mod, - Elf_Shdr *sechdrs, - unsigned int symindex, - unsigned int strindex, - const Elf_Ehdr *hdr, - const char *secstrings, - unsigned long *pstroffs, - unsigned long *strmap) +static void layout_symtab(struct module *mod, struct load_info *info) { - unsigned long symoffs; - Elf_Shdr *symsect = sechdrs + symindex; - Elf_Shdr *strsect = sechdrs + strindex; + Elf_Shdr *symsect = info->sechdrs + info->index.sym; + Elf_Shdr *strsect = info->sechdrs + info->index.str; const Elf_Sym *src; - const char *strtab; unsigned int i, nsrc, ndst; /* Put symbol section at end of init part of module. */ symsect->sh_flags |= SHF_ALLOC; symsect->sh_entsize = get_offset(mod, &mod->init_size, symsect, - symindex) | INIT_OFFSET_MASK; - DEBUGP("\t%s\n", secstrings + symsect->sh_name); + info->index.sym) | INIT_OFFSET_MASK; + DEBUGP("\t%s\n", info->secstrings + symsect->sh_name); - src = (void *)hdr + symsect->sh_offset; + src = (void *)info->hdr + symsect->sh_offset; nsrc = symsect->sh_size / sizeof(*src); - strtab = (void *)hdr + strsect->sh_offset; for (ndst = i = 1; i < nsrc; ++i, ++src) - if (is_core_symbol(src, sechdrs, hdr->e_shnum)) { + if (is_core_symbol(src, info->sechdrs, info->hdr->e_shnum)) { unsigned int j = src->st_name; - while(!__test_and_set_bit(j, strmap) && strtab[j]) + while (!__test_and_set_bit(j, info->strmap) + && info->strtab[j]) ++j; ++ndst; } /* Append room for core symbols at end of core part. */ - symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1); - mod->core_size = symoffs + ndst * sizeof(Elf_Sym); + info->symoffs = ALIGN(mod->core_size, symsect->sh_addralign ?: 1); + mod->core_size = info->symoffs + ndst * sizeof(Elf_Sym); /* Put string table section at end of init part of module. */ strsect->sh_flags |= SHF_ALLOC; strsect->sh_entsize = get_offset(mod, &mod->init_size, strsect, - strindex) | INIT_OFFSET_MASK; - DEBUGP("\t%s\n", secstrings + strsect->sh_name); + info->index.str) | INIT_OFFSET_MASK; + DEBUGP("\t%s\n", info->secstrings + strsect->sh_name); /* Append room for core symbols' strings at end of core part. */ - *pstroffs = mod->core_size; - __set_bit(0, strmap); - mod->core_size += bitmap_weight(strmap, strsect->sh_size); - - return symoffs; + info->stroffs = mod->core_size; + __set_bit(0, info->strmap); + mod->core_size += bitmap_weight(info->strmap, strsect->sh_size); } static void add_kallsyms(struct module *mod, struct load_info *info) @@ -2064,16 +2029,8 @@ static void add_kallsyms(struct module *mod, struct load_info *info) *++s = mod->strtab[i]; } #else -static inline unsigned long layout_symtab(struct module *mod, - Elf_Shdr *sechdrs, - unsigned int symindex, - unsigned int strindex, - const Elf_Ehdr *hdr, - const char *secstrings, - unsigned long *pstroffs, - unsigned long *strmap) +static inline void layout_symtab(struct module *mod, struct load_info *info) { - return 0; } static void add_kallsyms(struct module *mod, struct load_info *info) @@ -2113,30 +2070,28 @@ static void *module_alloc_update_bounds(unsigned long size) } #ifdef CONFIG_DEBUG_KMEMLEAK -static void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, - const Elf_Shdr *sechdrs, - const char *secstrings) +static void kmemleak_load_module(const struct module *mod, + const struct load_info *info) { unsigned int i; /* only scan the sections containing data */ kmemleak_scan_area(mod, sizeof(struct module), GFP_KERNEL); - for (i = 1; i < hdr->e_shnum; i++) { - if (!(sechdrs[i].sh_flags & SHF_ALLOC)) + for (i = 1; i < info->hdr->e_shnum; i++) { + const char *name = info->secstrings + info->sechdrs[i].sh_name; + if (!(info->sechdrs[i].sh_flags & SHF_ALLOC)) continue; - if (strncmp(secstrings + sechdrs[i].sh_name, ".data", 5) != 0 - && strncmp(secstrings + sechdrs[i].sh_name, ".bss", 4) != 0) + if (!strstarts(name, ".data") && !strstarts(name, ".bss")) continue; - kmemleak_scan_area((void *)sechdrs[i].sh_addr, - sechdrs[i].sh_size, GFP_KERNEL); + kmemleak_scan_area((void *)info->sechdrs[i].sh_addr, + info->sechdrs[i].sh_size, GFP_KERNEL); } } #else -static inline void kmemleak_load_module(struct module *mod, Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, - const char *secstrings) +static inline void kmemleak_load_module(const struct module *mod, + const struct load_info *info) { } #endif @@ -2227,8 +2182,8 @@ static int rewrite_section_headers(struct load_info *info) } /* Track but don't keep modinfo and version sections. */ - info->index.vers = find_sec(info->hdr, info->sechdrs, info->secstrings, "__versions"); - info->index.info = find_sec(info->hdr, info->sechdrs, info->secstrings, ".modinfo"); + info->index.vers = find_sec(info, "__versions"); + info->index.info = find_sec(info, ".modinfo"); info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; return 0; @@ -2268,8 +2223,7 @@ static struct module *setup_load_info(struct load_info *info) } } - info->index.mod = find_sec(info->hdr, info->sechdrs, info->secstrings, - ".gnu.linkonce.this_module"); + info->index.mod = find_sec(info, ".gnu.linkonce.this_module"); if (!info->index.mod) { printk(KERN_WARNING "No module found in object\n"); return ERR_PTR(-ENOEXEC); @@ -2283,7 +2237,7 @@ static struct module *setup_load_info(struct load_info *info) return ERR_PTR(-ENOEXEC); } - info->index.pcpu = find_pcpusec(info->hdr, info->sechdrs, info->secstrings); + info->index.pcpu = find_pcpusec(info); /* Check module struct version now, before we try to use module. */ if (!check_modstruct_version(info->sechdrs, info->index.vers, mod)) @@ -2292,11 +2246,9 @@ static struct module *setup_load_info(struct load_info *info) return mod; } -static int check_modinfo(struct module *mod, - const Elf_Shdr *sechdrs, - unsigned int infoindex, unsigned int versindex) +static int check_modinfo(struct module *mod, struct load_info *info) { - const char *modmagic = get_modinfo(sechdrs, infoindex, "vermagic"); + const char *modmagic = get_modinfo(info, "vermagic"); int err; /* This is allowed: modprobe --force will invalidate it. */ @@ -2304,13 +2256,13 @@ static int check_modinfo(struct module *mod, err = try_to_force_load(mod, "bad vermagic"); if (err) return err; - } else if (!same_magic(modmagic, vermagic, versindex)) { + } else if (!same_magic(modmagic, vermagic, info->index.vers)) { printk(KERN_ERR "%s: version magic '%s' should be '%s'\n", mod->name, modmagic, vermagic); return -ENOEXEC; } - if (get_modinfo(sechdrs, infoindex, "staging")) { + if (get_modinfo(info, "staging")) { add_taint_module(mod, TAINT_CRAP); printk(KERN_WARNING "%s: module is from the staging directory," " the quality is unknown, you have been warned.\n", @@ -2318,58 +2270,51 @@ static int check_modinfo(struct module *mod, } /* Set up license info based on the info section */ - set_license(mod, get_modinfo(sechdrs, infoindex, "license")); + set_license(mod, get_modinfo(info, "license")); return 0; } -static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, - Elf_Shdr *sechdrs, const char *secstrings) +static void find_module_sections(struct module *mod, + const struct load_info *info) { - mod->kp = section_objs(hdr, sechdrs, secstrings, "__param", + mod->kp = section_objs(info, "__param", sizeof(*mod->kp), &mod->num_kp); - mod->syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab", + mod->syms = section_objs(info, "__ksymtab", sizeof(*mod->syms), &mod->num_syms); - mod->crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab"); - mod->gpl_syms = section_objs(hdr, sechdrs, secstrings, "__ksymtab_gpl", + mod->crcs = section_addr(info, "__kcrctab"); + mod->gpl_syms = section_objs(info, "__ksymtab_gpl", sizeof(*mod->gpl_syms), &mod->num_gpl_syms); - mod->gpl_crcs = section_addr(hdr, sechdrs, secstrings, "__kcrctab_gpl"); - mod->gpl_future_syms = section_objs(hdr, sechdrs, secstrings, + mod->gpl_crcs = section_addr(info, "__kcrctab_gpl"); + mod->gpl_future_syms = section_objs(info, "__ksymtab_gpl_future", sizeof(*mod->gpl_future_syms), &mod->num_gpl_future_syms); - mod->gpl_future_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_gpl_future"); + mod->gpl_future_crcs = section_addr(info, "__kcrctab_gpl_future"); #ifdef CONFIG_UNUSED_SYMBOLS - mod->unused_syms = section_objs(hdr, sechdrs, secstrings, - "__ksymtab_unused", + mod->unused_syms = section_objs(info, "__ksymtab_unused", sizeof(*mod->unused_syms), &mod->num_unused_syms); - mod->unused_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_unused"); - mod->unused_gpl_syms = section_objs(hdr, sechdrs, secstrings, - "__ksymtab_unused_gpl", + mod->unused_crcs = section_addr(info, "__kcrctab_unused"); + mod->unused_gpl_syms = section_objs(info, "__ksymtab_unused_gpl", sizeof(*mod->unused_gpl_syms), &mod->num_unused_gpl_syms); - mod->unused_gpl_crcs = section_addr(hdr, sechdrs, secstrings, - "__kcrctab_unused_gpl"); + mod->unused_gpl_crcs = section_addr(info, "__kcrctab_unused_gpl"); #endif #ifdef CONFIG_CONSTRUCTORS - mod->ctors = section_objs(hdr, sechdrs, secstrings, ".ctors", + mod->ctors = section_objs(info, ".ctors", sizeof(*mod->ctors), &mod->num_ctors); #endif #ifdef CONFIG_TRACEPOINTS - mod->tracepoints = section_objs(hdr, sechdrs, secstrings, - "__tracepoints", + mod->tracepoints = section_objs(info, "__tracepoints", sizeof(*mod->tracepoints), &mod->num_tracepoints); #endif #ifdef CONFIG_EVENT_TRACING - mod->trace_events = section_objs(hdr, sechdrs, secstrings, - "_ftrace_events", + mod->trace_events = section_objs(info, "_ftrace_events", sizeof(*mod->trace_events), &mod->num_trace_events); /* @@ -2381,20 +2326,17 @@ static void find_module_sections(struct module *mod, Elf_Ehdr *hdr, #endif #ifdef CONFIG_FTRACE_MCOUNT_RECORD /* sechdrs[0].sh_size is always zero */ - mod->ftrace_callsites = section_objs(hdr, sechdrs, secstrings, - "__mcount_loc", + mod->ftrace_callsites = section_objs(info, "__mcount_loc", sizeof(*mod->ftrace_callsites), &mod->num_ftrace_callsites); #endif - if (section_addr(hdr, sechdrs, secstrings, "__obsparm")) + if (section_addr(info, "__obsparm")) printk(KERN_WARNING "%s: Ignoring obsolete parameters\n", mod->name); } -static int move_module(struct module *mod, - Elf_Ehdr *hdr, Elf_Shdr *sechdrs, - const char *secstrings, unsigned modindex) +static int move_module(struct module *mod, struct load_info *info) { int i; void *ptr; @@ -2430,32 +2372,31 @@ static int move_module(struct module *mod, /* Transfer each section which specifies SHF_ALLOC */ DEBUGP("final section addresses:\n"); - for (i = 0; i < hdr->e_shnum; i++) { + for (i = 0; i < info->hdr->e_shnum; i++) { void *dest; + Elf_Shdr *shdr = &info->sechdrs[i]; - if (!(sechdrs[i].sh_flags & SHF_ALLOC)) + if (!(shdr->sh_flags & SHF_ALLOC)) continue; - if (sechdrs[i].sh_entsize & INIT_OFFSET_MASK) + if (shdr->sh_entsize & INIT_OFFSET_MASK) dest = mod->module_init - + (sechdrs[i].sh_entsize & ~INIT_OFFSET_MASK); + + (shdr->sh_entsize & ~INIT_OFFSET_MASK); else - dest = mod->module_core + sechdrs[i].sh_entsize; + dest = mod->module_core + shdr->sh_entsize; - if (sechdrs[i].sh_type != SHT_NOBITS) - memcpy(dest, (void *)sechdrs[i].sh_addr, - sechdrs[i].sh_size); + if (shdr->sh_type != SHT_NOBITS) + memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size); /* Update sh_addr to point to copy in image. */ - sechdrs[i].sh_addr = (unsigned long)dest; + shdr->sh_addr = (unsigned long)dest; DEBUGP("\t0x%lx %s\n", - sechdrs[i].sh_addr, secstrings + sechdrs[i].sh_name); + shdr->sh_addr, info->secstrings + shdr->sh_name); } return 0; } -static int check_module_license_and_versions(struct module *mod, - Elf_Shdr *sechdrs) +static int check_module_license_and_versions(struct module *mod) { /* * ndiswrapper is under GPL by itself, but loads proprietary modules. @@ -2512,34 +2453,37 @@ static struct module *layout_and_allocate(struct load_info *info) { /* Module within temporary copy. */ struct module *mod; + Elf_Shdr *pcpusec; int err; mod = setup_load_info(info); if (IS_ERR(mod)) return mod; - err = check_modinfo(mod, info->sechdrs, info->index.info, info->index.vers); + err = check_modinfo(mod, info); if (err) return ERR_PTR(err); /* Allow arches to frob section contents and sizes. */ - err = module_frob_arch_sections(info->hdr, info->sechdrs, info->secstrings, mod); + err = module_frob_arch_sections(info->hdr, info->sechdrs, + info->secstrings, mod); if (err < 0) goto free_args; - if (info->index.pcpu) { + pcpusec = &info->sechdrs[info->index.pcpu]; + if (pcpusec->sh_size) { /* We have a special allocation for this section. */ - err = percpu_modalloc(mod, info->sechdrs[info->index.pcpu].sh_size, - info->sechdrs[info->index.pcpu].sh_addralign); + err = percpu_modalloc(mod, + pcpusec->sh_size, pcpusec->sh_addralign); if (err) goto free_args; - info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC; + pcpusec->sh_flags &= ~(unsigned long)SHF_ALLOC; } /* Determine total sizes, and put offsets in sh_entsize. For now this is done generically; there doesn't appear to be any special cases for the architectures. */ - layout_sections(mod, info->hdr, info->sechdrs, info->secstrings); + layout_sections(mod, info); info->strmap = kzalloc(BITS_TO_LONGS(info->sechdrs[info->index.str].sh_size) * sizeof(long), GFP_KERNEL); @@ -2547,17 +2491,16 @@ static struct module *layout_and_allocate(struct load_info *info) err = -ENOMEM; goto free_percpu; } - info->symoffs = layout_symtab(mod, info->sechdrs, info->index.sym, info->index.str, info->hdr, - info->secstrings, &info->stroffs, info->strmap); + layout_symtab(mod, info); /* Allocate and move to the final place */ - err = move_module(mod, info->hdr, info->sechdrs, info->secstrings, info->index.mod); + err = move_module(mod, info); if (err) goto free_strmap; /* Module has been copied to its final place now: return it. */ mod = (void *)info->sechdrs[info->index.mod].sh_addr; - kmemleak_load_module(mod, info->hdr, info->sechdrs, info->secstrings); + kmemleak_load_module(mod, info); return mod; free_strmap: @@ -2605,34 +2548,33 @@ static noinline struct module *load_module(void __user *umod, goto free_copy; } - /* Now we've moved module, initialize linked lists, etc. */ + /* Now module is in final location, initialize linked lists, etc. */ err = module_unload_init(mod); if (err) goto free_module; /* Now we've got everything in the final locations, we can * find optional sections. */ - find_module_sections(mod, info.hdr, info.sechdrs, info.secstrings); + find_module_sections(mod, &info); - err = check_module_license_and_versions(mod, info.sechdrs); + err = check_module_license_and_versions(mod); if (err) goto free_unload; /* Set up MODINFO_ATTR fields */ - setup_modinfo(mod, info.sechdrs, info.index.info); + setup_modinfo(mod, &info); /* Fix up syms, so that st_value is a pointer to location. */ - err = simplify_symbols(info.sechdrs, info.index.sym, info.strtab, info.index.vers, info.index.pcpu, - mod); + err = simplify_symbols(mod, &info); if (err < 0) goto free_modinfo; - err = apply_relocations(mod, info.hdr, info.sechdrs, info.index.sym, info.index.str); + err = apply_relocations(mod, &info); if (err < 0) goto free_modinfo; /* Set up and sort exception table */ - mod->extable = section_objs(info.hdr, info.sechdrs, info.secstrings, "__ex_table", + mod->extable = section_objs(&info, "__ex_table", sizeof(*mod->extable), &mod->num_exentries); sort_extable(mod->extable, mod->extable + mod->num_exentries); @@ -2643,7 +2585,7 @@ static noinline struct module *load_module(void __user *umod, add_kallsyms(mod, &info); if (!mod->taints) - debug = section_objs(info.hdr, info.sechdrs, info.secstrings, "__verbose", + debug = section_objs(&info, "__verbose", sizeof(*debug), &num_debug); err = module_finalize(info.hdr, info.sechdrs, mod); -- cgit v0.10.2 From 6526c534b2677ca601b7b92851437feb041d02a1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:10 -0600 Subject: module: move module args strndup_user to just before use Instead of copying and allocating the args and storing it in load_info, we can just allocate them right before we need them. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index 29dd232..cabafe2 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -114,7 +114,7 @@ struct load_info { Elf_Ehdr *hdr; unsigned long len; Elf_Shdr *sechdrs; - char *secstrings, *args, *strtab; + char *secstrings, *strtab; unsigned long *strmap; unsigned long symoffs, stroffs; struct { @@ -2096,7 +2096,7 @@ static inline void kmemleak_load_module(const struct module *mod, } #endif -/* Sets info->hdr, info->len and info->args. */ +/* Sets info->hdr and info->len. */ static int copy_and_check(struct load_info *info, const void __user *umod, unsigned long len, const char __user *uargs) @@ -2132,13 +2132,6 @@ static int copy_and_check(struct load_info *info, goto free_hdr; } - /* Now copy in args */ - info->args = strndup_user(uargs, ~0UL >> 1); - if (IS_ERR(info->args)) { - err = PTR_ERR(info->args); - goto free_hdr; - } - info->hdr = hdr; info->len = len; return 0; @@ -2150,7 +2143,6 @@ free_hdr: static void free_copy(struct load_info *info) { - kfree(info->args); vfree(info->hdr); } @@ -2468,7 +2460,7 @@ static struct module *layout_and_allocate(struct load_info *info) err = module_frob_arch_sections(info->hdr, info->sechdrs, info->secstrings, mod); if (err < 0) - goto free_args; + goto out; pcpusec = &info->sechdrs[info->index.pcpu]; if (pcpusec->sh_size) { @@ -2476,7 +2468,7 @@ static struct module *layout_and_allocate(struct load_info *info) err = percpu_modalloc(mod, pcpusec->sh_size, pcpusec->sh_addralign); if (err) - goto free_args; + goto out; pcpusec->sh_flags &= ~(unsigned long)SHF_ALLOC; } @@ -2507,8 +2499,7 @@ free_strmap: kfree(info->strmap); free_percpu: percpu_modfree(mod); -free_args: - kfree(info->args); +out: return ERR_PTR(err); } @@ -2594,7 +2585,12 @@ static noinline struct module *load_module(void __user *umod, flush_module_icache(mod); - mod->args = info.args; + /* Now copy in args */ + mod->args = strndup_user(uargs, ~0UL >> 1); + if (IS_ERR(mod->args)) { + err = PTR_ERR(mod->args); + goto free_arch_cleanup; + } mod->state = MODULE_STATE_COMING; @@ -2648,6 +2644,8 @@ static noinline struct module *load_module(void __user *umod, unlock: mutex_unlock(&module_mutex); synchronize_sched(); + kfree(mod->args); + free_arch_cleanup: module_arch_cleanup(mod); free_modinfo: free_modinfo(mod); -- cgit v0.10.2 From 811d66a0e1e99902d365497eec7884113a2665bd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:12 -0600 Subject: module: group post-relocation functions into post_relocation() This simply hoists more code out of load_module; we also put the identification of the extable and dynamic debug table in with the others in find_module_sections(). We move the taint check to the actual add/remove of the dynamic debug info: this is certain (find_module_sections is too early). Signed-off-by: Rusty Russell Cc: Yehuda Sadeh diff --git a/kernel/module.c b/kernel/module.c index cabafe2..f3cba93 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -117,6 +117,8 @@ struct load_info { char *secstrings, *strtab; unsigned long *strmap; unsigned long symoffs, stroffs; + struct _ddebug *debug; + unsigned int num_debug; struct { unsigned int sym, str, mod, vers, info, pcpu; } index; @@ -1993,7 +1995,7 @@ static void layout_symtab(struct module *mod, struct load_info *info) mod->core_size += bitmap_weight(info->strmap, strsect->sh_size); } -static void add_kallsyms(struct module *mod, struct load_info *info) +static void add_kallsyms(struct module *mod, const struct load_info *info) { unsigned int i, ndst; const Elf_Sym *src; @@ -2040,6 +2042,8 @@ static void add_kallsyms(struct module *mod, struct load_info *info) static void dynamic_debug_setup(struct _ddebug *debug, unsigned int num) { + if (!debug) + return; #ifdef CONFIG_DYNAMIC_DEBUG if (ddebug_add_module(debug, num, debug->modname)) printk(KERN_ERR "dynamic debug error adding module: %s\n", @@ -2267,8 +2271,7 @@ static int check_modinfo(struct module *mod, struct load_info *info) return 0; } -static void find_module_sections(struct module *mod, - const struct load_info *info) +static void find_module_sections(struct module *mod, struct load_info *info) { mod->kp = section_objs(info, "__param", sizeof(*mod->kp), &mod->num_kp); @@ -2323,9 +2326,15 @@ static void find_module_sections(struct module *mod, &mod->num_ftrace_callsites); #endif + mod->extable = section_objs(info, "__ex_table", + sizeof(*mod->extable), &mod->num_exentries); + if (section_addr(info, "__obsparm")) printk(KERN_WARNING "%s: Ignoring obsolete parameters\n", mod->name); + + info->debug = section_objs(info, "__verbose", + sizeof(*info->debug), &info->num_debug); } static int move_module(struct module *mod, struct load_info *info) @@ -2512,6 +2521,20 @@ static void module_deallocate(struct module *mod, struct load_info *info) module_free(mod, mod->module_core); } +static int post_relocation(struct module *mod, const struct load_info *info) +{ + sort_extable(mod->extable, mod->extable + mod->num_exentries); + + /* Copy relocated percpu area over. */ + percpu_modcopy(mod, (void *)info->sechdrs[info->index.pcpu].sh_addr, + info->sechdrs[info->index.pcpu].sh_size); + + add_kallsyms(mod, info); + + /* Arch-specific module finalizing. */ + return module_finalize(info->hdr, info->sechdrs, 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, @@ -2521,8 +2544,6 @@ static noinline struct module *load_module(void __user *umod, struct load_info info = { NULL, }; struct module *mod; long err; - struct _ddebug *debug = NULL; - unsigned int num_debug = 0; DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n", umod, len, uargs); @@ -2564,22 +2585,7 @@ static noinline struct module *load_module(void __user *umod, if (err < 0) goto free_modinfo; - /* Set up and sort exception table */ - mod->extable = section_objs(&info, "__ex_table", - sizeof(*mod->extable), &mod->num_exentries); - sort_extable(mod->extable, mod->extable + mod->num_exentries); - - /* Finally, copy percpu area over. */ - percpu_modcopy(mod, (void *)info.sechdrs[info.index.pcpu].sh_addr, - info.sechdrs[info.index.pcpu].sh_size); - - add_kallsyms(mod, &info); - - if (!mod->taints) - debug = section_objs(&info, "__verbose", - sizeof(*debug), &num_debug); - - err = module_finalize(info.hdr, info.sechdrs, mod); + err = post_relocation(mod, &info); if (err < 0) goto free_modinfo; @@ -2607,8 +2613,9 @@ static noinline struct module *load_module(void __user *umod, goto unlock; } - if (debug) - dynamic_debug_setup(debug, num_debug); + /* This has to be done once we're sure module name is unique. */ + if (!mod->taints) + dynamic_debug_setup(info.debug, info.num_debug); /* Find duplicate symbols */ err = verify_export_symbols(mod); @@ -2640,7 +2647,8 @@ static noinline struct module *load_module(void __user *umod, /* Unlink carefully: kallsyms could be walking list. */ list_del_rcu(&mod->list); ddebug: - dynamic_debug_remove(debug); + if (!mod->taints) + dynamic_debug_remove(info.debug); unlock: mutex_unlock(&module_mutex); synchronize_sched(); -- cgit v0.10.2 From 51f3d0f474aaebbc253100fa32a49c8256812330 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 5 Aug 2010 12:59:13 -0600 Subject: module: cleanup comments, remove noinline On my (32-bit x86) machine, sys_init_module() uses 124 bytes of stack once load_module() is inlined. This effectively reverts ffb4ba76 which inlined it due to stack pressure. Signed-off-by: Rusty Russell diff --git a/kernel/module.c b/kernel/module.c index f3cba93..d0b5f8d 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1,6 +1,6 @@ /* Copyright (C) 2002 Richard Henderson - Copyright (C) 2001 Rusty Russell, 2002 Rusty Russell IBM. + Copyright (C) 2001 Rusty Russell, 2002, 2010 Rusty Russell IBM. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -2523,12 +2523,14 @@ static void module_deallocate(struct module *mod, struct load_info *info) static int post_relocation(struct module *mod, const struct load_info *info) { + /* Sort exception table now relocations are done. */ sort_extable(mod->extable, mod->extable + mod->num_exentries); /* Copy relocated percpu area over. */ percpu_modcopy(mod, (void *)info->sechdrs[info->index.pcpu].sh_addr, info->sechdrs[info->index.pcpu].sh_size); + /* Setup kallsyms-specific fields. */ add_kallsyms(mod, info); /* Arch-specific module finalizing. */ @@ -2537,7 +2539,7 @@ static int post_relocation(struct module *mod, const struct load_info *info) /* 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, +static struct module *load_module(void __user *umod, unsigned long len, const char __user *uargs) { @@ -2598,6 +2600,7 @@ static noinline struct module *load_module(void __user *umod, goto free_arch_cleanup; } + /* Mark state as coming so strong_try_module_get() ignores us. */ mod->state = MODULE_STATE_COMING; /* Now sew it into the lists so we can get lockdep and oops @@ -2625,10 +2628,12 @@ static noinline struct module *load_module(void __user *umod, list_add_rcu(&mod->list, &modules); mutex_unlock(&module_mutex); + /* Module is ready to execute: parsing args may do that. */ err = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, NULL); if (err < 0) goto unlink; + /* Link in to syfs. */ err = mod_sysfs_setup(mod, &info, mod->kp, mod->num_kp); if (err < 0) goto unlink; @@ -2637,9 +2642,8 @@ static noinline struct module *load_module(void __user *umod, kfree(info.strmap); free_copy(&info); - trace_module_load(mod); - /* Done! */ + trace_module_load(mod); return mod; unlink: -- cgit v0.10.2