Messages in this thread Patch in this message | | | Date | Thu, 24 Oct 2019 12:59:04 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 15/16] module: Move where we mark modules RO,X |
| |
On Wed, Oct 23, 2019 at 12:15:14PM -0500, Josh Poimboeuf wrote: > On Wed, Oct 23, 2019 at 05:16:54PM +0200, Peter Zijlstra wrote: > > @@ -157,6 +158,14 @@ static int __apply_relocate_add(Elf64_Sh > > > > val = sym->st_value + rel[i].r_addend; > > > > + /* > > + * .klp.rela.* sections should only contain module > > + * related RELAs. All core-kernel RELAs should be in > > + * normal .rela.* sections and be applied when loading > > + * the patch module itself. > > + */ > > + WARN_ON_ONCE(klp && core_kernel_text(val)); > > + > > This isn't quite true, we also use .klp.rela sections to access > unexported vmlinux symbols.
Yes, you said in that earlier email. That all makes it really hard to validate this. But unless we validate it, it will stay buggy :/
Hmmm.... /me ponders
The alternative would be to apply the .klp.rela.* sections twice, once at patch-module load time and then apply those core_kernel_text() entries, and then once later and skip over them.
How's this?
--- --- a/arch/x86/kernel/module.c +++ b/arch/x86/kernel/module.c @@ -126,12 +126,20 @@ int apply_relocate(Elf32_Shdr *sechdrs, return 0; } #else /*X86_64*/ + +enum rela_filter { + rela_all = 0, + rela_core, + rela_mod, +}; + static int __apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab, unsigned int symindex, unsigned int relsec, struct module *me, - void *(*write)(void *addr, const void *val, size_t size)) + void *(*write)(void *addr, const void *val, size_t size), + enum rela_filter filter) { unsigned int i; Elf64_Rela *rel = (void *)sechdrs[relsec].sh_addr; @@ -157,6 +165,11 @@ static int __apply_relocate_add(Elf64_Sh val = sym->st_value + rel[i].r_addend; + if (filter) { + if ((filter == rela_core) != !!core_kernel_text(val)) + continue; + } + switch (ELF64_R_TYPE(rel[i].r_info)) { case R_X86_64_NONE: break; @@ -223,18 +236,21 @@ int apply_relocate_add(Elf64_Shdr *sechd unsigned int relsec, struct module *me) { - return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, memcpy); + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, memcpy, rela_all); } int klp_apply_relocate_add(Elf64_Shdr *sechdrs, const char *strtab, unsigned int symindex, unsigned int relsec, - struct module *me) + struct module *me, bool late) { int ret; - ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, text_poke); + if (!late) + return __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, memcpy, rela_core); + + ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me, text_poke, rela_mod); if (!ret) text_poke_sync(); --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -222,7 +222,7 @@ extern int klp_apply_relocate_add(Elf64_ const char *strtab, unsigned int symindex, unsigned int relsec, - struct module *me); + struct module *me, bool late); #else /* !CONFIG_LIVEPATCH */ --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -245,15 +245,6 @@ static int klp_resolve_symbols(Elf_Shdr return 0; } -int __weak klp_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); -} - static int klp_write_object_relocations(struct module *pmod, struct klp_object *obj) { @@ -296,7 +287,7 @@ static int klp_write_object_relocations( ret = klp_apply_relocate_add(pmod->klp_info->sechdrs, pmod->core_kallsyms.strtab, - pmod->klp_info->symndx, i, pmod); + pmod->klp_info->symndx, i, pmod, true); if (ret) break; } --- a/kernel/module.c +++ b/kernel/module.c @@ -2328,8 +2328,13 @@ static int apply_relocations(struct modu continue; /* Livepatch relocation sections are applied by livepatch */ - if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH) + if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH) { + if (info->sechdrs[i].sh_type == SHT_RELA) { + klp_apply_relocate_add(info->sechdrs, info->strtab, + info->index.sym, i, mod, false); + } continue; + } if (info->sechdrs[i].sh_type == SHT_REL) err = apply_relocate(info->sechdrs, info->strtab,
| |