Skip to content

Commit

Permalink
module: Don't ignore errors from set_memory_XX()
Browse files Browse the repository at this point in the history 
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: KSPP#7
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
  • Loading branch information
@chleroy @mcgrof
chleroy authored and mcgrof committed Feb 16, 2024
1 parent 1572853 commit d1909c0
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 22 deletions.
6 changes: 3 additions & 3 deletions kernel/module/internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
20 changes: 16 additions & 4 deletions kernel/module/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
51 changes: 36 additions & 15 deletions kernel/module/strict_rwx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/*
Expand All @@ -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,
Expand Down

0 comments on commit d1909c0

Please sign in to comment.