From 38f054d549a869f22a02224cd276a27bf14b6171 Mon Sep 17 00:00:00 2001 From: Jessica Yu Date: Tue, 23 Jul 2019 15:26:28 +0200 Subject: modules: always page-align module section allocations Some arches (e.g., arm64, x86) have moved towards non-executable module_alloc() allocations for security hardening reasons. That means that the module loader will need to set the text section of a module to executable, regardless of whether or not CONFIG_STRICT_MODULE_RWX is set. When CONFIG_STRICT_MODULE_RWX=y, module section allocations are always page-aligned to handle memory rwx permissions. On some arches with CONFIG_STRICT_MODULE_RWX=n however, when setting the module text to executable, the BUG_ON() in frob_text() gets triggered since module section allocations are not page-aligned when CONFIG_STRICT_MODULE_RWX=n. Since the set_memory_* API works with pages, and since we need to call set_memory_x() regardless of whether CONFIG_STRICT_MODULE_RWX is set, we might as well page-align all module section allocations for ease of managing rwx permissions of module sections (text, rodata, etc). Fixes: 2eef1399a866 ("modules: fix BUG when load module with rodata=n") Reported-by: Martin Kaiser Reported-by: Bartosz Golaszewski Tested-by: David Lechner Tested-by: Martin Kaiser Tested-by: Bartosz Golaszewski Signed-off-by: Jessica Yu --- kernel/module.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 5933395af9a0..cd8df516666d 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -64,14 +64,9 @@ /* * Modules' sections will be aligned on page boundaries - * to ensure complete separation of code and data, but - * only when CONFIG_STRICT_MODULE_RWX=y + * to ensure complete separation of code and data */ -#ifdef CONFIG_STRICT_MODULE_RWX # define debug_align(X) ALIGN(X, PAGE_SIZE) -#else -# define debug_align(X) (X) -#endif /* If this is set, the section belongs in the init part of the module */ #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) -- cgit v1.2.3 From c8424e776b093280d3fdd104d850706b3b229ac8 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 4 Jul 2019 15:57:34 -0300 Subject: MODSIGN: Export module signature definitions IMA will use the module_signature format for append signatures, so export the relevant definitions and factor out the code which verifies that the appended signature trailer is valid. Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it and be able to use mod_check_sig() without having to depend on either CONFIG_MODULE_SIG or CONFIG_MODULES. s390 duplicated the definition of struct module_signature so now they can use the new header instead. Signed-off-by: Thiago Jung Bauermann Acked-by: Jessica Yu Reviewed-by: Philipp Rudo Cc: Heiko Carstens Signed-off-by: Mimi Zohar --- kernel/module.c | 1 + 1 file changed, 1 insertion(+) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 5933395af9a0..5ac22efc3685 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include -- cgit v1.2.3 From 49fcf732bdae0550721ef73af7c45109ce26b2a9 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 19 Aug 2019 17:17:40 -0700 Subject: lockdown: Enforce module signatures if the kernel is locked down If the kernel is locked down, require that all modules have valid signatures that we can verify. I have adjusted the errors generated: (1) If there's no signature (ENODATA) or we can't check it (ENOPKG, ENOKEY), then: (a) If signatures are enforced then EKEYREJECTED is returned. (b) If there's no signature or we can't check it, but the kernel is locked down then EPERM is returned (this is then consistent with other lockdown cases). (2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we return the error we got. Note that the X.509 code doesn't check for key expiry as the RTC might not be valid or might not have been transferred to the kernel's clock yet. [Modified by Matthew Garrett to remove the IMA integration. This will be replaced with integration with the IMA architecture policy patchset.] Signed-off-by: David Howells Signed-off-by: Matthew Garrett Reviewed-by: Kees Cook Cc: Jessica Yu Signed-off-by: James Morris --- kernel/module.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 80c7c09584cf..2206c08a5e10 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2753,8 +2753,9 @@ static inline void kmemleak_load_module(const struct module *mod, #ifdef CONFIG_MODULE_SIG static int module_sig_check(struct load_info *info, int flags) { - int err = -ENOKEY; + int err = -ENODATA; const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1; + const char *reason; const void *mod = info->hdr; /* @@ -2769,16 +2770,38 @@ static int module_sig_check(struct load_info *info, int flags) err = mod_verify_sig(mod, info); } - if (!err) { + switch (err) { + case 0: info->sig_ok = true; return 0; - } - /* Not having a signature is only an error if we're strict. */ - if (err == -ENOKEY && !is_module_sig_enforced()) - err = 0; + /* We don't permit modules to be loaded into trusted kernels + * without a valid signature on them, but if we're not + * enforcing, certain errors are non-fatal. + */ + case -ENODATA: + reason = "Loading of unsigned module"; + goto decide; + case -ENOPKG: + reason = "Loading of module with unsupported crypto"; + goto decide; + case -ENOKEY: + reason = "Loading of module with unavailable key"; + decide: + if (is_module_sig_enforced()) { + pr_notice("%s is rejected\n", reason); + return -EKEYREJECTED; + } - return err; + return security_locked_down(LOCKDOWN_MODULE_SIGNATURE); + + /* All other errors are fatal, including nomem, unparseable + * signatures and signature check failures - even if signatures + * aren't required. + */ + default: + return err; + } } #else /* !CONFIG_MODULE_SIG */ static int module_sig_check(struct load_info *info, int flags) -- cgit v1.2.3 From 3b5be16c7e90a69c93349d210766250fffcb54bd Mon Sep 17 00:00:00 2001 From: He Zhe Date: Tue, 20 Aug 2019 22:53:10 +0800 Subject: modules: page-align module section allocations only for arches supporting strict module rwx We should keep the case of "#define debug_align(X) (X)" for all arches without CONFIG_HAS_STRICT_MODULE_RWX ability, which would save people, who are sensitive to system size, a lot of memory when using modules, especially for embedded systems. This is also the intention of the original #ifdef... statement and still valid for now. Note that this still keeps the effect of the fix of the following commit, 38f054d549a8 ("modules: always page-align module section allocations"), since when CONFIG_ARCH_HAS_STRICT_MODULE_RWX is enabled, module pages are aligned. Signed-off-by: He Zhe Signed-off-by: Jessica Yu --- kernel/module.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index cd8df516666d..9ee93421269c 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -64,9 +64,14 @@ /* * Modules' sections will be aligned on page boundaries - * to ensure complete separation of code and data + * to ensure complete separation of code and data, but + * only when CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y */ +#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX # define debug_align(X) ALIGN(X, PAGE_SIZE) +#else +# define debug_align(X) (X) +#endif /* If this is set, the section belongs in the init part of the module */ #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1)) -- cgit v1.2.3 From c5e4a062fe661806ab291a7576dc4a41613adb86 Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 6 Sep 2019 11:32:25 +0100 Subject: module: support reading multiple values per modinfo tag Similar to modpost's get_next_modinfo(), introduce get_next_modinfo() in kernel/module.c to acquire any further values associated with the same modinfo tag name. That is useful for any tags that have multiple occurrences (such as 'alias'), but is in particular introduced here as part of the symbol namespaces patch series to read the (potentially) multiple namespaces a module is importing. Reviewed-by: Joel Fernandes (Google) Reviewed-by: Martijn Coenen Reviewed-by: Greg Kroah-Hartman Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- kernel/module.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 9ee93421269c..3ee507c0a92f 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2481,7 +2481,8 @@ static char *next_string(char *string, unsigned long *secsize) return string; } -static char *get_modinfo(struct load_info *info, const char *tag) +static char *get_next_modinfo(const struct load_info *info, const char *tag, + char *prev) { char *p; unsigned int taglen = strlen(tag); @@ -2492,13 +2493,25 @@ static char *get_modinfo(struct load_info *info, const char *tag) * 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)) { + char *modinfo = (char *)info->hdr + infosec->sh_offset; + + if (prev) { + size -= prev - modinfo; + modinfo = next_string(prev, &size); + } + + for (p = modinfo; p; p = next_string(p, &size)) { if (strncmp(p, tag, taglen) == 0 && p[taglen] == '=') return p + taglen + 1; } return NULL; } +static char *get_modinfo(const struct load_info *info, const char *tag) +{ + return get_next_modinfo(info, tag, NULL); +} + static void setup_modinfo(struct module *mod, struct load_info *info) { struct module_attribute *attr; -- cgit v1.2.3 From 8651ec01daedad26290f76beeb4736f9d2da4b87 Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 6 Sep 2019 11:32:27 +0100 Subject: module: add support for symbol namespaces. The EXPORT_SYMBOL_NS() and EXPORT_SYMBOL_NS_GPL() macros can be used to export a symbol to a specific namespace. There are no _GPL_FUTURE and _UNUSED variants because these are currently unused, and I'm not sure they are necessary. I didn't add EXPORT_SYMBOL_NS() for ASM exports; this patch sets the namespace of ASM exports to NULL by default. In case of relative references, it will be relocatable to NULL. If there's a need, this should be pretty easy to add. A module that wants to use a symbol exported to a namespace must add a MODULE_IMPORT_NS() statement to their module code; otherwise, modpost will complain when building the module, and the kernel module loader will emit an error and fail when loading the module. MODULE_IMPORT_NS() adds a modinfo tag 'import_ns' to the module. That tag can be observed by the modinfo command, modpost and kernel/module.c at the time of loading the module. The ELF symbols are renamed to include the namespace with an asm label; for example, symbol 'usb_stor_suspend' in namespace USB_STORAGE becomes 'usb_stor_suspend.USB_STORAGE'. This allows modpost to do namespace checking, without having to go through all the effort of parsing ELF and relocation records just to get to the struct kernel_symbols. On x86_64 I saw no difference in binary size (compression), but at runtime this will require a word of memory per export to hold the namespace. An alternative could be to store namespaced symbols in their own section and use a separate 'struct namespaced_kernel_symbol' for that section, at the cost of making the module loader more complex. Co-developed-by: Martijn Coenen Signed-off-by: Martijn Coenen Reviewed-by: Greg Kroah-Hartman Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- kernel/module.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 3ee507c0a92f..6bb9b938f9c7 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -544,6 +544,15 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym) #endif } +static const char *kernel_symbol_namespace(const struct kernel_symbol *sym) +{ +#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS + return offset_to_ptr(&sym->namespace_offset); +#else + return sym->namespace; +#endif +} + static int cmp_name(const void *va, const void *vb) { const char *a; @@ -1379,6 +1388,34 @@ static inline int same_magic(const char *amagic, const char *bmagic, } #endif /* CONFIG_MODVERSIONS */ +static char *get_modinfo(const struct load_info *info, const char *tag); +static char *get_next_modinfo(const struct load_info *info, const char *tag, + char *prev); + +static int verify_namespace_is_imported(const struct load_info *info, + const struct kernel_symbol *sym, + struct module *mod) +{ + const char *namespace; + char *imported_namespace; + + namespace = kernel_symbol_namespace(sym); + if (namespace) { + imported_namespace = get_modinfo(info, "import_ns"); + while (imported_namespace) { + if (strcmp(namespace, imported_namespace) == 0) + return 0; + imported_namespace = get_next_modinfo( + info, "import_ns", imported_namespace); + } + pr_err("%s: module uses symbol (%s) from namespace %s, but does not import it.\n", + mod->name, kernel_symbol_name(sym), namespace); + return -EINVAL; + } + return 0; +} + + /* Resolve a symbol for this module. I.e. if we find one, record usage. */ static const struct kernel_symbol *resolve_symbol(struct module *mod, const struct load_info *info, @@ -1407,6 +1444,12 @@ static const struct kernel_symbol *resolve_symbol(struct module *mod, goto getname; } + err = verify_namespace_is_imported(info, sym, mod); + if (err) { + sym = ERR_PTR(err); + goto getname; + } + err = ref_module(mod, owner); if (err) { sym = ERR_PTR(err); -- cgit v1.2.3 From 3d52ec5e5d0dd7f8ca96a68c6756bd96e58b716b Mon Sep 17 00:00:00 2001 From: Matthias Maennich Date: Fri, 6 Sep 2019 11:32:29 +0100 Subject: module: add config option MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS If MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS is enabled (default=n), the requirement for modules to import all namespaces that are used by the module is relaxed. Enabling this option effectively allows (invalid) modules to be loaded while only a warning is emitted. Disabling this option keeps the enforcement at module loading time and loading is denied if the module's imports are not satisfactory. Reviewed-by: Martijn Coenen Reviewed-by: Greg Kroah-Hartman Signed-off-by: Matthias Maennich Signed-off-by: Jessica Yu --- kernel/module.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 6bb9b938f9c7..f76efcf2043e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1408,9 +1408,16 @@ static int verify_namespace_is_imported(const struct load_info *info, imported_namespace = get_next_modinfo( info, "import_ns", imported_namespace); } - pr_err("%s: module uses symbol (%s) from namespace %s, but does not import it.\n", - mod->name, kernel_symbol_name(sym), namespace); +#ifdef CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS + pr_warn( +#else + pr_err( +#endif + "%s: module uses symbol (%s) from namespace %s, but does not import it.\n", + mod->name, kernel_symbol_name(sym), namespace); +#ifndef CONFIG_MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS return -EINVAL; +#endif } return 0; } -- cgit v1.2.3 From 069e1c07c18ac2ccecdfb5ac287a37d6fb2d7a00 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Wed, 11 Sep 2019 13:26:46 +0100 Subject: module: Fix link failure due to invalid relocation on namespace offset Commit 8651ec01daed ("module: add support for symbol namespaces.") broke linking for arm64 defconfig: | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey': | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol' | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt': | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol' This is because the dummy initialisation of the 'namespace_offset' field in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with support for PREL32 locations uses an offset from an absolute address (0) in an effort to trick 'offset_to_pointer' into behaving as a NOP, allowing non-namespaced symbols to be treated in the same way as those belonging to a namespace. Unfortunately, place-relative relocations require a symbol reference rather than an absolute value and, although x86 appears to get away with this due to placing the kernel text at the top of the address space, it almost certainly results in a runtime failure if the kernel is relocated dynamically as a result of KASLR. Rework 'namespace_offset' so that a value of 0, which cannot occur for a valid namespaced symbol, indicates that the corresponding symbol does not belong to a namespace. Cc: Matthias Maennich Cc: Jessica Yu Cc: Ard Biesheuvel Cc: Catalin Marinas Fixes: 8651ec01daed ("module: add support for symbol namespaces.") Reported-by: kbuild test robot Tested-by: Matthias Maennich Tested-by: Ard Biesheuvel Reviewed-by: Matthias Maennich Acked-by: Ard Biesheuvel Signed-off-by: Will Deacon Signed-off-by: Jessica Yu --- kernel/module.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index f76efcf2043e..7ab244c4e1ba 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -547,6 +547,8 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym) static const char *kernel_symbol_namespace(const struct kernel_symbol *sym) { #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS + if (!sym->namespace_offset) + return NULL; return offset_to_ptr(&sym->namespace_offset); #else return sym->namespace; -- cgit v1.2.3 From b605be658188005efd2d447e0989fd1f4aab8862 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Mon, 9 Sep 2019 20:39:02 +0900 Subject: module: remove unneeded casts in cmp_name() You can pass opaque pointers directly. I also renamed 'va' and 'vb' into more meaningful arguments. Signed-off-by: Masahiro Yamada Signed-off-by: Jessica Yu --- kernel/module.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'kernel/module.c') diff --git a/kernel/module.c b/kernel/module.c index 7ab244c4e1ba..32873bcce738 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -555,12 +555,9 @@ static const char *kernel_symbol_namespace(const struct kernel_symbol *sym) #endif } -static int cmp_name(const void *va, const void *vb) +static int cmp_name(const void *name, const void *sym) { - const char *a; - const struct kernel_symbol *b; - a = va; b = vb; - return strcmp(a, kernel_symbol_name(b)); + return strcmp(name, kernel_symbol_name(sym)); } static bool find_exported_symbol_in_section(const struct symsearch *syms, -- cgit v1.2.3