Messages in this thread | | | Date | Mon, 28 Oct 2019 11:07:21 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH v4 15/16] module: Move where we mark modules RO,X |
| |
On Fri, Oct 25, 2019 at 08:17:41PM -0500, Josh Poimboeuf wrote:
> + * The following restrictions apply to module-specific relocation sections: > + * > + * a) References to vmlinux symbols are not allowed. Otherwise there might > + * be module init ordering issues, and crashes might occur in some of the > + * other kernel patching components like paravirt patching or jump > + * labels. All references to vmlinux symbols should use either normal > + * relas (for exported symbols) or vmlinux-specific klp relas (for > + * unexported symbols). This restriction is enforced in > + * klp_resolve_symbols().
Right.
> + * b) Relocations to special sections like __jump_table and .altinstructions > + * aren't allowed. In other words, there should never be a > + * .klp.rela.{module}.__jump_table section. This will definitely cause > + * initialization ordering issues, as such special sections are processed > + * during the loading of the klp module itself, *not* the to-be-patched > + * module. This means that e.g., it's not currently possible to patch a > + * module function which uses a static key jump label, if you want to > + * have the replacement function also use the same static key. In this > + * case, a non-static interface like static_key_enabled() can be used in > + * the new function instead.
Idem for .static_call_sites I suppose..
Is there any enforcement on this? I'm thinking it should be possible to detect the presence of these sections and yell a bit.
OTOH, it should be possible to actually handle this, but let's do that later.
> + * On the other hand, a .klp.rela.vmlinux.__jump_table section is fine, > + * as it can be resolved early enough during the load of the klp module, > + * as described above. > + */
> diff --git a/kernel/module.c b/kernel/module.c > index fe5bd382759c..ff4347385f05 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -2327,11 +2327,9 @@ static int apply_relocations(struct module *mod, const struct load_info *info) > if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC)) > continue; > > - /* Livepatch relocation sections are applied by livepatch */ > if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH) > - continue; > - > - if (info->sechdrs[i].sh_type == SHT_REL) > + err = klp_write_relocations(mod, NULL); > + else if (info->sechdrs[i].sh_type == SHT_REL) > err = apply_relocate(info->sechdrs, info->strtab, > info->index.sym, i, mod); > else if (info->sechdrs[i].sh_type == SHT_RELA)
Like here, we can yell and error if .klp.rela.{mod}.__jump_table sections are encountered.
Other than that, this should work I suppose.
| |