[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191024105904.GB4131@hirez.programming.kicks-ass.net>
Date: Thu, 24 Oct 2019 12:59:04 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
mhiramat@...nel.org, bristot@...hat.com, jbaron@...mai.com,
torvalds@...ux-foundation.org, tglx@...utronix.de,
mingo@...nel.org, namit@...are.com, hpa@...or.com, luto@...nel.org,
ard.biesheuvel@...aro.org, jeyu@...nel.org
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,
Powered by blists - more mailing lists