As pointed out by Sean [1], is_kernel_noinstr_text() will return false for an address contained within a module's .noinstr.text section. A later patch will require checking whether a text address is noinstr, and this can unfortunately be the case of modules - KVM is one such case.
A module's .noinstr.text section is already tracked as of commit 66e9b0717102 ("kprobes: Prevent probes in .noinstr.text section") for kprobe blacklisting purposes, but via an ad-hoc mechanism.
Add a MOD_NOINSTR_TEXT mem_type, and reorganize __layout_sections() so that it maps all the sections in a single invocation.
[1]: http://lore.kernel.org/r/Z4qQL89GZ_gk0vpu@google.com Signed-off-by: Valentin Schneider vschneid@redhat.com --- include/linux/module.h | 6 ++-- kernel/kprobes.c | 8 ++--- kernel/module/main.c | 76 ++++++++++++++++++++++++++++++++---------- 3 files changed, 66 insertions(+), 24 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h index d94b196d5a34e..193d8d34eeee0 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -332,6 +332,7 @@ struct mod_tree_node {
enum mod_mem_type { MOD_TEXT = 0, + MOD_NOINSTR_TEXT, MOD_DATA, MOD_RODATA, MOD_RO_AFTER_INIT, @@ -502,8 +503,6 @@ struct module { void __percpu *percpu; unsigned int percpu_size; #endif - void *noinstr_text_start; - unsigned int noinstr_text_size;
#ifdef CONFIG_TRACEPOINTS unsigned int num_tracepoints; @@ -622,12 +621,13 @@ static inline bool module_is_coming(struct module *mod) return mod->state == MODULE_STATE_COMING; }
-struct module *__module_text_address(unsigned long addr); struct module *__module_address(unsigned long addr); +struct module *__module_text_address(unsigned long addr); bool is_module_address(unsigned long addr); bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr); bool is_module_percpu_address(unsigned long addr); bool is_module_text_address(unsigned long addr); +bool is_module_noinstr_text_address(unsigned long addr);
static inline bool within_module_mem_type(unsigned long addr, const struct module *mod, diff --git a/kernel/kprobes.c b/kernel/kprobes.c index ffe0c3d523063..9a799faee68a1 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2547,9 +2547,9 @@ static void add_module_kprobe_blacklist(struct module *mod) kprobe_add_area_blacklist(start, end); }
- start = (unsigned long)mod->noinstr_text_start; + start = (unsigned long)mod->mem[MOD_NOINSTR_TEXT].base; if (start) { - end = start + mod->noinstr_text_size; + end = start + mod->mem[MOD_NOINSTR_TEXT].size; kprobe_add_area_blacklist(start, end); } } @@ -2570,9 +2570,9 @@ static void remove_module_kprobe_blacklist(struct module *mod) kprobe_remove_area_blacklist(start, end); }
- start = (unsigned long)mod->noinstr_text_start; + start = (unsigned long)mod->mem[MOD_NOINSTR_TEXT].base; if (start) { - end = start + mod->noinstr_text_size; + end = start + mod->mem[MOD_NOINSTR_TEXT].size; kprobe_remove_area_blacklist(start, end); } } diff --git a/kernel/module/main.c b/kernel/module/main.c index b9f010daaa4c7..0126bae64b698 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1558,7 +1558,17 @@ bool module_init_layout_section(const char *sname) return module_init_section(sname); }
-static void __layout_sections(struct module *mod, struct load_info *info, bool is_init) +static bool module_noinstr_layout_section(const char *sname) +{ + return strstarts(sname, ".noinstr"); +} + +static bool module_default_layout_section(const char *sname) +{ + return !module_init_layout_section(sname) && !module_noinstr_layout_section(sname); +} + +static void __layout_sections(struct module *mod, struct load_info *info) { unsigned int m, i;
@@ -1567,20 +1577,44 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i * Mask of excluded section header flags } */ static const unsigned long masks[][2] = { + /* Core */ + { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, + { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, + { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, + { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, + { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, + { ARCH_SHF_SMALL | SHF_ALLOC, 0 }, + /* Init */ { SHF_EXECINSTR | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_ALLOC, SHF_WRITE | ARCH_SHF_SMALL }, { SHF_RO_AFTER_INIT | SHF_ALLOC, ARCH_SHF_SMALL }, { SHF_WRITE | SHF_ALLOC, ARCH_SHF_SMALL }, - { ARCH_SHF_SMALL | SHF_ALLOC, 0 } + { ARCH_SHF_SMALL | SHF_ALLOC, 0 }, }; - static const int core_m_to_mem_type[] = { + static bool (*const section_filter[])(const char *) = { + /* Core */ + module_default_layout_section, + module_noinstr_layout_section, + module_default_layout_section, + module_default_layout_section, + module_default_layout_section, + module_default_layout_section, + /* Init */ + module_init_layout_section, + module_init_layout_section, + module_init_layout_section, + module_init_layout_section, + module_init_layout_section, + }; + static const int mem_type_map[] = { + /* Core */ MOD_TEXT, + MOD_NOINSTR_TEXT, MOD_RODATA, MOD_RO_AFTER_INIT, MOD_DATA, MOD_DATA, - }; - static const int init_m_to_mem_type[] = { + /* Init */ MOD_INIT_TEXT, MOD_INIT_RODATA, MOD_INVALID, @@ -1589,16 +1623,16 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i };
for (m = 0; m < ARRAY_SIZE(masks); ++m) { - enum mod_mem_type type = is_init ? init_m_to_mem_type[m] : core_m_to_mem_type[m]; + enum mod_mem_type type = mem_type_map[m];
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 - || is_init != module_init_layout_section(sname)) + if ((s->sh_flags & masks[m][0]) != masks[m][0] || + (s->sh_flags & masks[m][1]) || + s->sh_entsize != ~0UL || + !section_filter[m](sname)) continue;
if (WARN_ON_ONCE(type == MOD_INVALID)) @@ -1638,10 +1672,7 @@ static void layout_sections(struct module *mod, struct load_info *info) info->sechdrs[i].sh_entsize = ~0UL;
pr_debug("Core section allocation order for %s:\n", mod->name); - __layout_sections(mod, info, false); - - pr_debug("Init section allocation order for %s:\n", mod->name); - __layout_sections(mod, info, true); + __layout_sections(mod, info); }
static void module_license_taint_check(struct module *mod, const char *license) @@ -2515,9 +2546,6 @@ static int find_module_sections(struct module *mod, struct load_info *info) } #endif
- mod->noinstr_text_start = section_objs(info, ".noinstr.text", 1, - &mod->noinstr_text_size); - #ifdef CONFIG_TRACEPOINTS mod->tracepoints_ptrs = section_objs(info, "__tracepoints_ptrs", sizeof(*mod->tracepoints_ptrs), @@ -3769,12 +3797,26 @@ struct module *__module_text_address(unsigned long addr) if (mod) { /* Make sure it's within the text section. */ if (!within_module_mem_type(addr, mod, MOD_TEXT) && + !within_module_mem_type(addr, mod, MOD_NOINSTR_TEXT) && !within_module_mem_type(addr, mod, MOD_INIT_TEXT)) mod = NULL; } return mod; }
+bool is_module_noinstr_text_address(unsigned long addr) +{ + scoped_guard(preempt) { + struct module *mod = __module_address(addr); + + /* Make sure it's within the .noinstr.text section. */ + if (mod) + return within_module_mem_type(addr, mod, MOD_NOINSTR_TEXT); + } + + return false; +} + /* Don't grab lock, we're oopsing. */ void print_modules(void) {