| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5cb55af6-5c9f-4a75-8c42-819415aec4d2@samsung.com>
Date: Fri, 16 Feb 2024 12:33:03 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>, Luis Chamberlain
<mcgrof@...nel.org>, linux-modules@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Kees Cook <keescook@...omium.org>,
"linux-hardening @ vger . kernel . org" <linux-hardening@...r.kernel.org>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v2] module: Don't ignore errors from set_memory_XX()
On 16.02.2024 09:14, Christophe Leroy wrote:
> set_memory_ro(), set_memory_nx(), set_memory_x() and other helpers
> can fail and return an error. In that case the memory might not be
> protected as expected and the module loading has to be aborted to
> avoid security issues.
>
> Check return value of all calls to set_memory_XX() and handle
> error if any.
>
> Add a check to not call set_memory_XX() on NULL pointers as some
> architectures may not like it allthough numpages is always 0 in that
> case. This also avoid a useless call to set_vm_flush_reset_perms().
>
> Link:https://github.com/KSPP/linux/issues/7
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
> ---
> v2:
> - Bail out early from module_set_memory() when address is NULL.
> - Properly clear bug list when module_set_memory() fails in complete_formation().
>
> This patch applies on top of modules/modules-next branch
> ---
> kernel/module/internal.h | 6 ++---
> kernel/module/main.c | 20 ++++++++++++---
> kernel/module/strict_rwx.c | 51 +++++++++++++++++++++++++++-----------
> 3 files changed, 55 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 4f1b98f011da..2ebece8a789f 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -322,9 +322,9 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root *
> }
> #endif /* CONFIG_MODULES_TREE_LOOKUP */
>
> -void module_enable_rodata_ro(const struct module *mod, bool after_init);
> -void module_enable_data_nx(const struct module *mod);
> -void module_enable_text_rox(const struct module *mod);
> +int module_enable_rodata_ro(const struct module *mod, bool after_init);
> +int module_enable_data_nx(const struct module *mod);
> +int module_enable_text_rox(const struct module *mod);
> int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> char *secstrings, struct module *mod);
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index a9a4a4885102..689def7676c4 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2571,7 +2571,9 @@ static noinline int do_init_module(struct module *mod)
> /* Switch to core kallsyms now init is done: kallsyms may be walking! */
> rcu_assign_pointer(mod->kallsyms, &mod->core_kallsyms);
> #endif
> - module_enable_rodata_ro(mod, true);
> + ret = module_enable_rodata_ro(mod, true);
> + if (ret)
> + goto fail_mutex_unlock;
> mod_tree_remove_init(mod);
> module_arch_freeing_init(mod);
> for_class_mod_mem_type(type, init) {
> @@ -2609,6 +2611,8 @@ static noinline int do_init_module(struct module *mod)
>
> return 0;
>
> +fail_mutex_unlock:
> + mutex_unlock(&module_mutex);
> fail_free_freeinit:
> kfree(freeinit);
> fail:
> @@ -2736,9 +2740,15 @@ static int complete_formation(struct module *mod, struct load_info *info)
> module_bug_finalize(info->hdr, info->sechdrs, mod);
> module_cfi_finalize(info->hdr, info->sechdrs, mod);
>
> - module_enable_rodata_ro(mod, false);
> - module_enable_data_nx(mod);
> - module_enable_text_rox(mod);
> + err = module_enable_rodata_ro(mod, false);
> + if (err)
> + goto out_strict_rwx;
> + err = module_enable_data_nx(mod);
> + if (err)
> + goto out_strict_rwx;
> + err = module_enable_text_rox(mod);
> + if (err)
> + goto out_strict_rwx;
>
> /*
> * Mark state as coming so strong_try_module_get() ignores us,
> @@ -2749,6 +2759,8 @@ static int complete_formation(struct module *mod, struct load_info *info)
>
> return 0;
>
> +out_strict_rwx:
> + module_bug_cleanup(mod);
> out:
> mutex_unlock(&module_mutex);
> return err;
> diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
> index b36d93983465..c45caa4690e5 100644
> --- a/kernel/module/strict_rwx.c
> +++ b/kernel/module/strict_rwx.c
> @@ -11,13 +11,16 @@
> #include <linux/set_memory.h>
> #include "internal.h"
>
> -static void module_set_memory(const struct module *mod, enum mod_mem_type type,
> - int (*set_memory)(unsigned long start, int num_pages))
> +static int module_set_memory(const struct module *mod, enum mod_mem_type type,
> + int (*set_memory)(unsigned long start, int num_pages))
> {
> const struct module_memory *mod_mem = &mod->mem[type];
>
> + if (!mod_mem->base)
> + return 0;
> +
> set_vm_flush_reset_perms(mod_mem->base);
> - set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT);
> + return set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT);
> }
>
> /*
> @@ -26,35 +29,53 @@ static void module_set_memory(const struct module *mod, enum mod_mem_type type,
> * CONFIG_STRICT_MODULE_RWX because they are needed regardless of whether we
> * are strict.
> */
> -void module_enable_text_rox(const struct module *mod)
> +int module_enable_text_rox(const struct module *mod)
> {
> for_class_mod_mem_type(type, text) {
> + int ret;
> +
> if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
> - module_set_memory(mod, type, set_memory_rox);
> + ret = module_set_memory(mod, type, set_memory_rox);
> else
> - module_set_memory(mod, type, set_memory_x);
> + ret = module_set_memory(mod, type, set_memory_x);
> + if (ret)
> + return ret;
> }
> + return 0;
> }
>
> -void module_enable_rodata_ro(const struct module *mod, bool after_init)
> +int module_enable_rodata_ro(const struct module *mod, bool after_init)
> {
> + int ret;
> +
> if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX) || !rodata_enabled)
> - return;
> + return 0;
>
> - module_set_memory(mod, MOD_RODATA, set_memory_ro);
> - module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro);
> + ret = module_set_memory(mod, MOD_RODATA, set_memory_ro);
> + if (ret)
> + return ret;
> + ret = module_set_memory(mod, MOD_INIT_RODATA, set_memory_ro);
> + if (ret)
> + return ret;
>
> if (after_init)
> - module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro);
> + return module_set_memory(mod, MOD_RO_AFTER_INIT, set_memory_ro);
> +
> + return 0;
> }
>
> -void module_enable_data_nx(const struct module *mod)
> +int module_enable_data_nx(const struct module *mod)
> {
> if (!IS_ENABLED(CONFIG_STRICT_MODULE_RWX))
> - return;
> + return 0;
>
> - for_class_mod_mem_type(type, data)
> - module_set_memory(mod, type, set_memory_nx);
> + for_class_mod_mem_type(type, data) {
> + int ret = module_set_memory(mod, type, set_memory_nx);
> +
> + if (ret)
> + return ret;
> + }
> + return 0;
> }
>
> int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists