From 9f2d1e68cf4d641def734adaccfc3823d3575e6c Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 5 Jun 2018 10:22:52 +0200 Subject: module: exclude SHN_UNDEF symbols from kallsyms api Livepatch modules are special in that we preserve their entire symbol tables in order to be able to apply relocations after module load. The unwanted side effect of this is that undefined (SHN_UNDEF) symbols of livepatch modules are accessible via the kallsyms api and this can confuse symbol resolution in livepatch (klp_find_object_symbol()) and cause subtle bugs in livepatch. Have the module kallsyms api skip over SHN_UNDEF symbols. These symbols are usually not available for normal modules anyway as we cut down their symbol tables to just the core (non-undefined) symbols, so this should really just affect livepatch modules. Note that this patch doesn't affect the display of undefined symbols in /proc/kallsyms. Reported-by: Josh Poimboeuf Tested-by: Josh Poimboeuf Reviewed-by: Josh Poimboeuf Signed-off-by: Jessica Yu --- kernel/module.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index f475f30eed8c..4a6b9c6d5f2c 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -4067,7 +4067,7 @@ static unsigned long mod_find_symname(struct module *mod, const char *name) for (i = 0; i < kallsyms->num_symtab; i++) if (strcmp(name, symname(kallsyms, i)) == 0 && - kallsyms->symtab[i].st_info != 'U') + kallsyms->symtab[i].st_shndx != SHN_UNDEF) return kallsyms->symtab[i].st_value; return 0; } @@ -4113,6 +4113,10 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *, if (mod->state == MODULE_STATE_UNFORMED) continue; for (i = 0; i < kallsyms->num_symtab; i++) { + + if (kallsyms->symtab[i].st_shndx == SHN_UNDEF) + continue; + ret = fn(data, symname(kallsyms, i), mod, kallsyms->symtab[i].st_value); if (ret != 0) -- cgit From 81a0abd9f213704fbeeea1550ff202000e3c3cdf Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Fri, 22 Jun 2018 13:59:29 +0200 Subject: module: make it clear when we're handling the module copy in info->hdr In load_module(), it's not always clear whether we're handling the temporary module copy in info->hdr (which is freed at the end of load_module()) or if we're handling the module already allocated and copied to it's final place. Adding an info->mod field and using it whenever we're handling the temporary copy makes that explicitly clear. Signed-off-by: Jessica Yu --- kernel/module.c | 42 +++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 4a6b9c6d5f2c..3ed4aaa646dc 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -309,6 +309,8 @@ EXPORT_SYMBOL(unregister_module_notifier); struct load_info { const char *name; + /* pointer to module in temporary copy, freed at end of load_module() */ + struct module *mod; Elf_Ehdr *hdr; unsigned long len; Elf_Shdr *sechdrs; @@ -2947,14 +2949,13 @@ static int rewrite_section_headers(struct load_info *info, int flags) * 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). + * Set info->mod to the temporary copy of the module in info->hdr. The final one + * will be allocated in move_module(). */ -static struct module *setup_load_info(struct load_info *info, int flags) +static int setup_load_info(struct load_info *info, int flags) { unsigned int i; int err; - struct module *mod; /* Set up the convenience variables */ info->sechdrs = (void *)info->hdr + info->hdr->e_shoff; @@ -2963,7 +2964,7 @@ static struct module *setup_load_info(struct load_info *info, int flags) err = rewrite_section_headers(info, flags); if (err) - return ERR_PTR(err); + return err; /* Find internal symbols and strings. */ for (i = 1; i < info->hdr->e_shnum; i++) { @@ -2980,30 +2981,30 @@ static struct module *setup_load_info(struct load_info *info, int flags) if (!info->index.mod) { pr_warn("%s: No module found in object\n", info->name ?: "(missing .modinfo name field)"); - return ERR_PTR(-ENOEXEC); + return -ENOEXEC; } /* This is temporary: point mod into copy of data. */ - mod = (void *)info->sechdrs[info->index.mod].sh_addr; + info->mod = (void *)info->sechdrs[info->index.mod].sh_addr; /* * If we didn't load the .modinfo 'name' field, fall back to * on-disk struct mod 'name' field. */ if (!info->name) - info->name = mod->name; + info->name = info->mod->name; if (info->index.sym == 0) { pr_warn("%s: module has no symbols (stripped?)\n", info->name); - return ERR_PTR(-ENOEXEC); + return -ENOEXEC; } info->index.pcpu = find_pcpusec(info); /* Check module struct version now, before we try to use module. */ - if (!check_modstruct_version(info, mod)) - return ERR_PTR(-ENOEXEC); + if (!check_modstruct_version(info, info->mod)) + return -ENOEXEC; - return mod; + return 0; } static int check_modinfo(struct module *mod, struct load_info *info, int flags) @@ -3298,25 +3299,24 @@ core_param(module_blacklist, module_blacklist, charp, 0400); static struct module *layout_and_allocate(struct load_info *info, int flags) { - /* Module within temporary copy. */ struct module *mod; unsigned int ndx; int err; - mod = setup_load_info(info, flags); - if (IS_ERR(mod)) - return mod; + err = setup_load_info(info, flags); + if (err) + return ERR_PTR(err); if (blacklisted(info->name)) return ERR_PTR(-EPERM); - err = check_modinfo(mod, info, flags); + err = check_modinfo(info->mod, info, flags); 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); + info->secstrings, info->mod); if (err < 0) return ERR_PTR(err); @@ -3335,11 +3335,11 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) /* 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); - layout_symtab(mod, info); + layout_sections(info->mod, info); + layout_symtab(info->mod, info); /* Allocate and move to the final place */ - err = move_module(mod, info); + err = move_module(info->mod, info); if (err) return ERR_PTR(err); -- cgit From 5fdc7db6448a4f558f298b1c98d6d124fdf2ad95 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Fri, 22 Jun 2018 14:00:01 +0200 Subject: module: setup load info before module_sig_check() We want to be able to log the module name in early error messages, such as when module signature verification fails. Previously, the module name is set in layout_and_allocate(), meaning that any error messages that happen before (such as those in module_sig_check()) won't be logged with a module name, which isn't terribly helpful. In order to do this, reshuffle the order in load_module() and set up load info earlier so that we can log the module name along with these error messages. This requires splitting rewrite_section_headers() out of setup_load_info(). While we're at it, clean up and split up the operations done in layout_and_allocate(), setup_load_info(), and rewrite_section_headers() more cleanly so these functions only perform what their names suggest. Signed-off-by: Jessica Yu --- kernel/module.c | 77 ++++++++++++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 3ed4aaa646dc..0ad0bb58e116 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2488,7 +2488,11 @@ static char *get_modinfo(struct load_info *info, const char *tag) Elf_Shdr *infosec = &info->sechdrs[info->index.info]; unsigned long size = infosec->sh_size; - for (p = (char *)infosec->sh_addr; p; p = next_string(p, &size)) { + /* + * get_modinfo() calls made before rewrite_section_headers() + * must use sh_offset, as sh_addr isn't set! + */ + for (p = (char *)info->hdr + infosec->sh_offset; p; p = next_string(p, &size)) { if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=') return p + taglen + 1; } @@ -2928,17 +2932,7 @@ static int rewrite_section_headers(struct load_info *info, int flags) } /* Track but don't keep modinfo and version sections. */ - if (flags & MODULE_INIT_IGNORE_MODVERSIONS) - info->index.vers = 0; /* Pretend no __versions section! */ - else - info->index.vers = find_sec(info, "__versions"); info->sechdrs[info->index.vers].sh_flags &= ~(unsigned long)SHF_ALLOC; - - info->index.info = find_sec(info, ".modinfo"); - if (!info->index.info) - info->name = "(missing .modinfo section)"; - else - info->name = get_modinfo(info, "name"); info->sechdrs[info->index.info].sh_flags &= ~(unsigned long)SHF_ALLOC; return 0; @@ -2955,16 +2949,18 @@ static int rewrite_section_headers(struct load_info *info, int flags) static int setup_load_info(struct load_info *info, int flags) { unsigned int i; - int err; /* 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; - err = rewrite_section_headers(info, flags); - if (err) - return err; + /* Try to find a name early so we can log errors with a module name */ + info->index.info = find_sec(info, ".modinfo"); + if (!info->index.info) + info->name = "(missing .modinfo section)"; + else + info->name = get_modinfo(info, "name"); /* Find internal symbols and strings. */ for (i = 1; i < info->hdr->e_shnum; i++) { @@ -2977,6 +2973,11 @@ static int setup_load_info(struct load_info *info, int flags) } } + if (info->index.sym == 0) { + pr_warn("%s: module has no symbols (stripped?)\n", info->name); + return -ENOEXEC; + } + info->index.mod = find_sec(info, ".gnu.linkonce.this_module"); if (!info->index.mod) { pr_warn("%s: No module found in object\n", @@ -2984,26 +2985,22 @@ static int setup_load_info(struct load_info *info, int flags) return -ENOEXEC; } /* This is temporary: point mod into copy of data. */ - info->mod = (void *)info->sechdrs[info->index.mod].sh_addr; + info->mod = (void *)info->hdr + info->sechdrs[info->index.mod].sh_offset; /* - * If we didn't load the .modinfo 'name' field, fall back to + * If we didn't load the .modinfo 'name' field earlier, fall back to * on-disk struct mod 'name' field. */ if (!info->name) info->name = info->mod->name; - if (info->index.sym == 0) { - pr_warn("%s: module has no symbols (stripped?)\n", info->name); - return -ENOEXEC; - } + if (flags & MODULE_INIT_IGNORE_MODVERSIONS) + info->index.vers = 0; /* Pretend no __versions section! */ + else + info->index.vers = find_sec(info, "__versions"); info->index.pcpu = find_pcpusec(info); - /* Check module struct version now, before we try to use module. */ - if (!check_modstruct_version(info, info->mod)) - return -ENOEXEC; - return 0; } @@ -3303,13 +3300,6 @@ static struct module *layout_and_allocate(struct load_info *info, int flags) unsigned int ndx; int err; - err = setup_load_info(info, flags); - if (err) - return ERR_PTR(err); - - if (blacklisted(info->name)) - return ERR_PTR(-EPERM); - err = check_modinfo(info->mod, info, flags); if (err) return ERR_PTR(err); @@ -3657,17 +3647,36 @@ static int load_module(struct load_info *info, const char __user *uargs, int flags) { struct module *mod; - long err; + long err = 0; char *after_dashes; + err = elf_header_check(info); + if (err) + goto free_copy; + + err = setup_load_info(info, flags); + if (err) + goto free_copy; + + if (blacklisted(info->name)) { + err = -EPERM; + goto free_copy; + } + err = module_sig_check(info, flags); if (err) goto free_copy; - err = elf_header_check(info); + err = rewrite_section_headers(info, flags); if (err) goto free_copy; + /* Check module struct version now, before we try to use module. */ + if (!check_modstruct_version(info, info->mod)) { + err = -ENOEXEC; + goto free_copy; + } + /* Figure out module layout, and allocate all the memory. */ mod = layout_and_allocate(info, flags); if (IS_ERR(mod)) { -- cgit From 62267e0ecc9c00a1b8ff7859cfa03e34b419f7ee Mon Sep 17 00:00:00 2001 From: "Jason A. Donenfeld" Date: Fri, 22 Jun 2018 17:38:50 +0200 Subject: module: print sensible error code Printing "err 0" to the user in the warning message is not particularly useful, especially when this gets transformed into a -ENOENT for the remainder of the call chain. Signed-off-by: Jason A. Donenfeld Signed-off-by: Jessica Yu --- kernel/module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 0ad0bb58e116..b6b3a3c58af1 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2284,9 +2284,9 @@ static int simplify_symbols(struct module *mod, const struct load_info *info) if (!ksym && ELF_ST_BIND(sym[i].st_info) == STB_WEAK) break; - pr_warn("%s: Unknown symbol %s (err %li)\n", - mod->name, name, PTR_ERR(ksym)); ret = PTR_ERR(ksym) ?: -ENOENT; + pr_warn("%s: Unknown symbol %s (err %d)\n", + mod->name, name, ret); break; default: -- cgit From 996302c5e85650722f1e5aeaeaaac12f9f362bf8 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Sun, 24 Jun 2018 00:37:44 +0900 Subject: module: replace VMLINUX_SYMBOL_STR() with __stringify() or string literal With the special case handling for Blackfin and Metag was removed by commit 94e58e0ac312 ("export.h: remove code for prefixing symbols with underscore"), VMLINUX_SYMBOL_STR() is now equivalent to __stringify(). Replace the remaining usages to prepare for the entire removal of VMLINUX_SYMBOL_STR(). Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- include/linux/module.h | 4 ++-- kernel/module.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index d44df9b2c131..f807f15bebbe 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -266,7 +266,7 @@ extern int modules_disabled; /* for sysctl */ /* Get/put a kernel symbol (calls must be symmetric) */ void *__symbol_get(const char *symbol); void *__symbol_get_gpl(const char *symbol); -#define symbol_get(x) ((typeof(&x))(__symbol_get(VMLINUX_SYMBOL_STR(x)))) +#define symbol_get(x) ((typeof(&x))(__symbol_get(__stringify(x)))) /* modules using other modules: kdb wants to see this. */ struct module_use { @@ -575,7 +575,7 @@ extern void __noreturn __module_put_and_exit(struct module *mod, #ifdef CONFIG_MODULE_UNLOAD int module_refcount(struct module *mod); void __symbol_put(const char *symbol); -#define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) +#define symbol_put(x) __symbol_put(__stringify(x)) void symbol_put_addr(void *addr); /* Sometimes we know we already have a refcount, and it's easier not diff --git a/kernel/module.c b/kernel/module.c index b6b3a3c58af1..ba45a84e4287 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1341,14 +1341,12 @@ static inline int check_modstruct_version(const struct load_info *info, * locking is necessary -- use preempt_disable() to placate lockdep. */ preempt_disable(); - if (!find_symbol(VMLINUX_SYMBOL_STR(module_layout), NULL, - &crc, true, false)) { + if (!find_symbol("module_layout", NULL, &crc, true, false)) { preempt_enable(); BUG(); } preempt_enable(); - return check_version(info, VMLINUX_SYMBOL_STR(module_layout), - mod, crc); + return check_version(info, "module_layout", mod, crc); } /* First part is kernel version, which we ignore if module has crcs. */ -- cgit From f314dfea16a085a58d2ff227ea9fa9e490ee5d18 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Fri, 29 Jun 2018 16:37:08 +0200 Subject: modsign: log module name in the event of an error Now that we have the load_info struct all initialized (including info->name, which contains the name of the module) before module_sig_check(), make the load_info struct and hence module name available to mod_verify_sig() so that we can log the module name in the event of an error. Signed-off-by: Jessica Yu --- kernel/module-internal.h | 25 ++++++++++++++++++++++++- kernel/module.c | 22 +--------------------- kernel/module_signing.c | 12 +++++++----- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/kernel/module-internal.h b/kernel/module-internal.h index 915e123a430f..79c9be2dbbe9 100644 --- a/kernel/module-internal.h +++ b/kernel/module-internal.h @@ -9,4 +9,27 @@ * 2 of the Licence, or (at your option) any later version. */ -extern int mod_verify_sig(const void *mod, unsigned long *_modlen); +#include +#include + +struct load_info { + const char *name; + /* pointer to module in temporary copy, freed at end of load_module() */ + struct module *mod; + Elf_Ehdr *hdr; + unsigned long len; + Elf_Shdr *sechdrs; + char *secstrings, *strtab; + unsigned long symoffs, stroffs; + struct _ddebug *debug; + unsigned int num_debug; + bool sig_ok; +#ifdef CONFIG_KALLSYMS + unsigned long mod_kallsyms_init_off; +#endif + struct { + unsigned int sym, str, mod, vers, info, pcpu; + } index; +}; + +extern int mod_verify_sig(const void *mod, struct load_info *info); diff --git a/kernel/module.c b/kernel/module.c index ba45a84e4287..8a45986fd728 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -307,26 +307,6 @@ int unregister_module_notifier(struct notifier_block *nb) } EXPORT_SYMBOL(unregister_module_notifier); -struct load_info { - const char *name; - /* pointer to module in temporary copy, freed at end of load_module() */ - struct module *mod; - Elf_Ehdr *hdr; - unsigned long len; - Elf_Shdr *sechdrs; - char *secstrings, *strtab; - unsigned long symoffs, stroffs; - struct _ddebug *debug; - unsigned int num_debug; - bool sig_ok; -#ifdef CONFIG_KALLSYMS - unsigned long mod_kallsyms_init_off; -#endif - struct { - unsigned int sym, str, mod, vers, info, pcpu; - } index; -}; - /* * We require a truly strong try_module_get(): 0 means success. * Otherwise an error is returned due to ongoing or failed @@ -2778,7 +2758,7 @@ static int module_sig_check(struct load_info *info, int flags) memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) { /* We truncate the module to discard the signature */ info->len -= markerlen; - err = mod_verify_sig(mod, &info->len); + err = mod_verify_sig(mod, info); } if (!err) { diff --git a/kernel/module_signing.c b/kernel/module_signing.c index 937c844bee4a..f2075ce8e4b3 100644 --- a/kernel/module_signing.c +++ b/kernel/module_signing.c @@ -45,10 +45,10 @@ struct module_signature { /* * Verify the signature on a module. */ -int mod_verify_sig(const void *mod, unsigned long *_modlen) +int mod_verify_sig(const void *mod, struct load_info *info) { struct module_signature ms; - size_t modlen = *_modlen, sig_len; + size_t sig_len, modlen = info->len; pr_devel("==>%s(,%zu)\n", __func__, modlen); @@ -62,10 +62,11 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen) if (sig_len >= modlen) return -EBADMSG; modlen -= sig_len; - *_modlen = modlen; + info->len = modlen; if (ms.id_type != PKEY_ID_PKCS7) { - pr_err("Module is not signed with expected PKCS#7 message\n"); + pr_err("%s: Module is not signed with expected PKCS#7 message\n", + info->name); return -ENOPKG; } @@ -76,7 +77,8 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen) ms.__pad[0] != 0 || ms.__pad[1] != 0 || ms.__pad[2] != 0) { - pr_err("PKCS#7 signature info has unexpected non-zero params\n"); + pr_err("%s: PKCS#7 signature info has unexpected non-zero params\n", + info->name); return -EBADMSG; } -- cgit From 4d58e7034d1971436d44f203cf69d2feb6b82e5c Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Fri, 6 Jul 2018 14:48:47 +0200 Subject: ARM: module: fix modsign build error The asm/module.h header file can not be included standalone, which breaks the module signing code after a recent change: In file included from kernel/module-internal.h:13, from kernel/module_signing.c:17: arch/arm/include/asm/module.h:37:27: error: 'struct module' declared inside parameter list will not be visible outside of this definition or declaration [-Werror] u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val); This adds a forward declaration of struct module to make it all work. Fixes: f314dfea16a0 ("modsign: log module name in the event of an error") Signed-off-by: Arnd Bergmann Acked-by: Russell King Signed-off-by: Jessica Yu --- arch/arm/include/asm/module.h | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h index 89ad0596033a..9e81b7c498d8 100644 --- a/arch/arm/include/asm/module.h +++ b/arch/arm/include/asm/module.h @@ -34,6 +34,7 @@ struct mod_arch_specific { #endif }; +struct module; u32 get_module_plt(struct module *mod, unsigned long loc, Elf32_Addr val); /* -- cgit From 9be936f4b3a2ec101f54cff9cf1a6abf67263c50 Mon Sep 17 00:00:00 2001 From: zhong jiang Date: Wed, 1 Aug 2018 00:56:17 +0800 Subject: kernel/module: Use kmemdup to replace kmalloc+memcpy we prefer to the kmemdup rather than kmalloc+memcpy. so just replace them. Signed-off-by: zhong jiang Signed-off-by: Jessica Yu --- kernel/module.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 8a45986fd728..54fbac81fd56 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2039,21 +2039,19 @@ static int copy_module_elf(struct module *mod, struct load_info *info) /* Elf section header table */ size = sizeof(*info->sechdrs) * info->hdr->e_shnum; - mod->klp_info->sechdrs = kmalloc(size, GFP_KERNEL); + mod->klp_info->sechdrs = kmemdup(info->sechdrs, size, GFP_KERNEL); if (mod->klp_info->sechdrs == NULL) { ret = -ENOMEM; goto free_info; } - memcpy(mod->klp_info->sechdrs, info->sechdrs, size); /* Elf section name string table */ size = info->sechdrs[info->hdr->e_shstrndx].sh_size; - mod->klp_info->secstrings = kmalloc(size, GFP_KERNEL); + mod->klp_info->secstrings = kmemdup(info->secstrings, size, GFP_KERNEL); if (mod->klp_info->secstrings == NULL) { ret = -ENOMEM; goto free_sechdrs; } - memcpy(mod->klp_info->secstrings, info->secstrings, size); /* Elf symbol section index */ symndx = info->index.sym; -- cgit