Re: [RFC PATCH v3 4/6] livepatch: reuse module loader code to write relocations
From: Petr Mladek <hidden>
Date: 2016-01-11 16:56:21
Also in:
lkml
On Fri 2016-01-08 14:28:22, Jessica Yu wrote:
quoted hunk ↗ jump to hunk
Reuse module loader code to write relocations, thereby eliminating the need for architecture specific relocation code in livepatch. Namely, we reuse apply_relocate_add() in the module loader to write relocations instead of duplicating functionality in livepatch's klp_write_module_reloc(). To apply relocation sections, remaining SHN_LIVEPATCH symbols referenced by relocs are resolved and then apply_relocate_add() is called to apply those relocations. In addition, remove x86 livepatch relocation code. It is no longer needed since symbol resolution and relocation work have been offloaded to module loader.--- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c@@ -204,74 +207,70 @@ static int klp_find_object_symbol(const char *objname, const char *name, return -EINVAL; } -/* - * external symbols are located outside the parent object (where the parent - * object is either vmlinux or the kmod being patched). - */ -static int klp_find_external_symbol(struct module *pmod, const char *name, - unsigned long *addr) +static int klp_resolve_symbols(Elf_Shdr *relsec, struct module *pmod) { - const struct kernel_symbol *sym; + int i, len, ret = 0; + Elf_Rela *relas; + Elf_Sym *sym; + char *symname, *sym_objname; - /* first, check if it's an exported symbol */ - preempt_disable(); - sym = find_symbol(name, NULL, NULL, true, true); - if (sym) { - *addr = sym->value; - preempt_enable(); - return 0; + relas = (Elf_Rela *) relsec->sh_addr; + /* For each rela in this .klp.rel. section */ + for (i = 0; i < relsec->sh_size / sizeof(Elf_Rela); i++) { + sym = pmod->core_symtab + ELF_R_SYM(relas[i].r_info); + symname = pmod->core_strtab + sym->st_name; + + len = strcspn(symname + KLP_TAG_LEN, ".");
We should check that len is non-zero. Otherwise, sym_objname might be empty string and symname might overflow below. Also we should check that symname really starts with .klp.sym. to avoid invalid memory access.
+ sym_objname = strncmp(symname + KLP_TAG_LEN, "vmlinux", len) ?
+ kstrndup(symname + KLP_TAG_LEN, len, GFP_KERNEL) : NULL;
+ /* .klp.sym.objname.[symbol_name] */
+ symname += KLP_TAG_LEN + len + 1;
+
+ ret = klp_find_object_symbol(sym_objname, symname,
+ KLP_SYMPOS(sym->st_other),
+ (unsigned long *) &sym->st_value);
+ kfree(sym_objname);
+ if (ret)
+ return ret;
}
- preempt_enable();
- /*
- * Check if it's in another .o within the patch module. This also
- * checks that the external symbol is unique.
- */
- return klp_find_object_symbol(pmod->name, name, 0, addr);
+ return ret;
}
static int klp_write_object_relocations(struct module *pmod,
struct klp_object *obj)
{
- int ret = 0;
- unsigned long val;
- struct klp_reloc *reloc;
+ int i, len, ret = 0;
+ char *secname;
+ const char *objname;
if (WARN_ON(!klp_is_object_loaded(obj)))
return -EINVAL;
- if (WARN_ON(!obj->relocs))
- return -EINVAL;
+ objname = klp_is_module(obj) ? obj->name : "vmlinux";
module_disable_ro(pmod);
+ /* For each klp rela section for this object */
+ for (i = 1; i < pmod->info->hdr->e_shnum; i++) {
+ if (!(pmod->info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH))
+ continue;
+
+ secname = pmod->info->secstrings + pmod->info->sechdrs[i].sh_name;
+ /* .klp.rel.[objname].section_name */
+ len = strcspn(secname + KLP_TAG_LEN, ".");We should check that secname realy starts with .klp.rel. string to avoid access out of memmory. Otherwise, nice work. Thanks, Petr.
+
+ if (strncmp(objname, secname + KLP_TAG_LEN, len))
+ continue;
- for (reloc = obj->relocs; reloc->name; reloc++) {
- /* discover the address of the referenced symbol */
- if (reloc->external) {
- if (reloc->sympos > 0) {
- pr_err("non-zero sympos for external reloc symbol '%s' is not supported\n",
- reloc->name);
- ret = -EINVAL;
- goto out;
- }
- ret = klp_find_external_symbol(pmod, reloc->name, &val);
- } else
- ret = klp_find_object_symbol(obj->name,
- reloc->name,
- reloc->sympos,
- &val);
+ ret = klp_resolve_symbols(pmod->info->sechdrs + i, pmod);
if (ret)
goto out;
- ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
- val + reloc->addend);
- if (ret) {
- pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
- reloc->name, val, ret);
+ ret = apply_relocate_add(pmod->info->sechdrs, pmod->core_strtab,
+ pmod->info->index.sym, i, pmod);
+ if (ret)
goto out;
- }
}
-
out:
module_enable_ro(pmod);
return ret;