Messages in this thread Patch in this message | | | Date | Mon, 21 Oct 2019 17:34:25 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 15/16] module: Move where we mark modules RO,X |
| |
On Mon, Oct 21, 2019 at 04:14:02PM +0200, Peter Zijlstra wrote: > On Mon, Oct 21, 2019 at 08:53:12AM -0500, Josh Poimboeuf wrote: > > On Fri, Oct 18, 2019 at 09:35:40AM +0200, Peter Zijlstra wrote: > > > Now that set_all_modules_text_*() is gone, nothing depends on the > > > relation between ->state = COMING and the protection state anymore. > > > This enables moving the protection changes later, such that the COMING > > > notifier callbacks can more easily modify the text. > > > > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > > Cc: Jessica Yu <jeyu@kernel.org> > > > --- > > > kernel/module.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > --- a/kernel/module.c > > > +++ b/kernel/module.c > > > @@ -3683,10 +3683,6 @@ static int complete_formation(struct mod > > > /* This relies on module_mutex for list integrity. */ > > > module_bug_finalize(info->hdr, info->sechdrs, mod); > > > > > > - module_enable_ro(mod, false); > > > - module_enable_nx(mod); > > > - module_enable_x(mod); > > > - > > > /* Mark state as coming so strong_try_module_get() ignores us, > > > * but kallsyms etc. can see us. */ > > > mod->state = MODULE_STATE_COMING; > > > @@ -3852,6 +3848,10 @@ static int load_module(struct load_info > > > if (err) > > > goto bug_cleanup; > > > > > > + module_enable_ro(mod, false); > > > + module_enable_nx(mod); > > > + module_enable_x(mod); > > > + > > > /* Module is ready to execute: parsing args may do that. */ > > > after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, > > > -32768, 32767, mod, > > > > [ Sorry if this was already discussed, I still have a large backlog. ] > > > > Doesn't livepatch code also need to be modified? We have: > > Urgh bah.. I was too focussed on the other klp borkage :/ But yes, > arm64/ftrace and klp are the only two users of that function (outside of > module.c) and Mark was already writing a patch for arm64. > > Means these last two patches need to wait a little until we've fixed > those.
So On IRC Josh suggested we use text_poke() for RELA. Since KLP is only available on Power and x86, and Power does not have STRICT_MODULE_RWX, the below should be sufficient.
Completely untested...
--- arch/x86/kernel/module.c | 40 +++++++++++++++++++++++++++++++++------- include/linux/livepatch.h | 7 +++++++ kernel/livepatch/core.c | 14 ++++++++++---- 3 files changed, 50 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c index d5c72cb877b3..76fa2c5f2d7b 100644 --- a/arch/x86/kernel/module.c +++ b/arch/x86/kernel/module.c @@ -126,11 +126,12 @@ int apply_relocate(Elf32_Shdr *sechdrs, return 0; } #else /*X86_64*/ -int apply_relocate_add(Elf64_Shdr *sechdrs, +int __apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab, unsigned int symindex, unsigned int relsec, - struct module *me) + struct module *me, + void *(*write)(void *addr, const void *val, size_t size)) { unsigned int i; Elf64_Rela *rel = (void *)sechdrs[relsec].sh_addr; @@ -162,19 +163,19 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, case R_X86_64_64: if (*(u64 *)loc != 0) goto invalid_relocation; - *(u64 *)loc = val; + write(loc, &val, 8); break; case R_X86_64_32: if (*(u32 *)loc != 0) goto invalid_relocation; - *(u32 *)loc = val; + write(loc, &val, 4); if (val != *(u32 *)loc) goto overflow; break; case R_X86_64_32S: if (*(s32 *)loc != 0) goto invalid_relocation; - *(s32 *)loc = val; + write(loc, &val, 4); if ((s64)val != *(s32 *)loc) goto overflow; break; @@ -183,7 +184,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, if (*(u32 *)loc != 0) goto invalid_relocation; val -= (u64)loc; - *(u32 *)loc = val; + write(loc, &val, 4); #if 0 if ((s64)val != *(s32 *)loc) goto overflow; @@ -193,7 +194,7 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, if (*(u64 *)loc != 0) goto invalid_relocation; val -= (u64)loc; - *(u64 *)loc = val; + write(loc, &val, 8); break; default: pr_err("%s: Unknown rela relocation: %llu\n", @@ -215,6 +216,31 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, me->name); return -ENOEXEC; } + +int apply_relocate_add(Elf64_Shdr *sechdrs, + const char *strtab, + unsigned int symindex, + unsigned int relsec, + struct module *me) +{ + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, memcpy); +} + +int klp_apply_relocate_add(Elf64_Shdr *sechdrs, + const char *strtab, + unsigned int symindex, + unsigned int relsec, + struct module *me) +{ + int ret; + + ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, text_poke); + if (!ret) + text_poke_sync(); + + return ret; +} + #endif int module_finalize(const Elf_Ehdr *hdr, diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 273400814020..5b8c10871b70 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -217,6 +217,13 @@ void *klp_shadow_get_or_alloc(void *obj, unsigned long id, void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor); void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor); + +extern int klp_apply_relocate_add(Elf64_Shdr *sechdrs, + const char *strtab, + unsigned int symindex, + unsigned int relsec, + struct module *me); + #else /* !CONFIG_LIVEPATCH */ static inline int klp_module_coming(struct module *mod) { return 0; } diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index ab4a4606d19b..e690519aba31 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -245,6 +245,15 @@ static int klp_resolve_symbols(Elf_Shdr *relasec, struct module *pmod) return 0; } +int __weak klp_apply_relocate_add(Elf64_Shdr *sechdrs, + const char *strtab, + unsigned int symindex, + unsigned int relsec, + struct module *me) +{ + apply_relocate_add(sechdrs, strtab, symindex, relsec, me); +} + static int klp_write_object_relocations(struct module *pmod, struct klp_object *obj) { @@ -285,7 +294,7 @@ static int klp_write_object_relocations(struct module *pmod, if (ret) break; - ret = apply_relocate_add(pmod->klp_info->sechdrs, + ret = klp_apply_relocate_add(pmod->klp_info->sechdrs, pmod->core_kallsyms.strtab, pmod->klp_info->symndx, i, pmod); if (ret) @@ -721,16 +730,13 @@ static int klp_init_object_loaded(struct klp_patch *patch, mutex_lock(&text_mutex); - module_disable_ro(patch->mod); ret = klp_write_object_relocations(patch->mod, obj); if (ret) { - module_enable_ro(patch->mod, true); mutex_unlock(&text_mutex); return ret; } arch_klp_init_object_loaded(patch, obj); - module_enable_ro(patch->mod, true); mutex_unlock(&text_mutex);
| |