On Mon, Nov 09, 2015 at 11:45:53PM -0500, Jessica Yu wrote:
> Reuse module loader code to write relocations, thereby eliminating the
> need for architecture specific code in livepatch. Namely, we reuse
> apply_relocate_add() in the module loader to write relocs 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.
>
> Signed-off-by: Jessica Yu <j...@redhat.com>
> ---
> include/linux/livepatch.h | 11 ++++--
> include/linux/module.h | 6 ++++
> kernel/livepatch/core.c | 89
> +++++++++++++++++++++++++++++------------------
> 3 files changed, 70 insertions(+), 36 deletions(-)
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 31db7a0..601e892 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -85,7 +85,7 @@ struct klp_reloc {
> /**
> * struct klp_object - kernel object structure for live patching
> * @name: module name (or NULL for vmlinux)
> - * @relocs: relocation entries to be applied at load time
> + * @reloc_secs: relocation sections to be applied at load time
> * @funcs: function entries for functions to be patched in the object
> * @kobj: kobject for sysfs resources
> * @mod: kernel module associated with the patched object
> @@ -95,7 +95,7 @@ struct klp_reloc {
> struct klp_object {
> /* external */
> const char *name;
> - struct klp_reloc *relocs;
> + struct list_head reloc_secs;
> struct klp_func *funcs;
>
> /* internal */
> @@ -129,6 +129,13 @@ struct klp_patch {
> #define klp_for_each_func(obj, func) \
> for (func = obj->funcs; func->old_name; func++)
>
> +struct klp_reloc_sec {
> + unsigned int index;
> + char *name;
> + char *objname;
> + struct list_head list;
> +};
> +
> int klp_register_patch(struct klp_patch *);
> int klp_unregister_patch(struct klp_patch *);
> int klp_enable_patch(struct klp_patch *);
> diff --git a/include/linux/module.h b/include/linux/module.h
> index c8680b1..3c34eb8 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -793,9 +793,15 @@ extern int module_sysfs_initialized;
> #ifdef CONFIG_DEBUG_SET_MODULE_RONX
> extern void set_all_modules_text_rw(void);
> extern void set_all_modules_text_ro(void);
> +extern void
> +set_page_attributes(void *start, void *end,
> + int (*set)(unsigned long start, int num_pages));
> #else
> static inline void set_all_modules_text_rw(void) { }
> static inline void set_all_modules_text_ro(void) { }
> +static inline void
> +set_page_attributes(void *start, void *end,
> + int (*set)(unsigned long start, int num_pages)) { }
> #endif
>
> #ifdef CONFIG_GENERIC_BUG
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 087a8c7..26c419f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -28,6 +28,8 @@
> #include <linux/list.h>
> #include <linux/kallsyms.h>
> #include <linux/livepatch.h>
> +#include <linux/elf.h>
> +#include <asm/cacheflush.h>
>
> /**
> * struct klp_ops - structure for tracking registered ftrace ops structs
> @@ -281,46 +283,54 @@ static int klp_find_external_symbol(struct module
> *pmod, const char *name,
> }
>
> static int klp_write_object_relocations(struct module *pmod,
> - struct klp_object *obj)
> + struct klp_object *obj,
> + struct klp_patch *patch)
> {
> - int ret;
> - struct klp_reloc *reloc;
> + int relindex, num_relas;
> + int i, ret = 0;
> + unsigned long addr;
> + unsigned int bind;
> + char *symname;
> + struct klp_reloc_sec *reloc_sec;
> + struct load_info *info;
> + Elf_Rela *rela;
> + Elf_Sym *sym, *symtab;
> + Elf_Shdr *symsect;
>
> if (WARN_ON(!klp_is_object_loaded(obj)))
> return -EINVAL;
>
> - if (WARN_ON(!obj->relocs))
> - return -EINVAL;
> -
> - for (reloc = obj->relocs; reloc->name; reloc++) {
> - if (!klp_is_module(obj)) {
> - ret = klp_verify_vmlinux_symbol(reloc->name,
> - reloc->val);
> - if (ret)
> - return ret;
> - } else {
> - /* module, reloc->val needs to be discovered */
> - if (reloc->external)
> - ret = klp_find_external_symbol(pmod,
> - reloc->name,
> - &reloc->val);
> - else
> - ret = klp_find_object_symbol(obj->mod->name,
> - reloc->name,
> - &reloc->val);
> - if (ret)
> - return ret;
> - }
> - ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> - reloc->val + reloc->addend);
> - if (ret) {
> - pr_err("relocation failed for symbol '%s' at 0x%016lx
> (%d)\n",
> - reloc->name, reloc->val, ret);
> - return ret;
> + info = pmod->info;
> + symsect = info->sechdrs + info->index.sym;
> + symtab = (void *)info->hdr + symsect->sh_offset;
> +
> + /* For each __klp_rela section for this object */
> + list_for_each_entry(reloc_sec, &obj->reloc_secs, list) {
> + relindex = reloc_sec->index;
> + num_relas = info->sechdrs[relindex].sh_size / sizeof(Elf_Rela);
> + rela = (Elf_Rela *) info->sechdrs[relindex].sh_addr;
> +
> + /* For each rela in this __klp_rela section */
> + for (i = 0; i < num_relas; i++, rela++) {
> + sym = symtab + ELF_R_SYM(rela->r_info);
> + symname = info->strtab + sym->st_name;
> + bind = ELF_ST_BIND(sym->st_info);
> +
> + if (sym->st_shndx == SHN_LIVEPATCH) {
> + if (bind == STB_LIVEPATCH_EXT)
> + ret = klp_find_external_symbol(pmod,
> symname, &addr);
> + else
> + ret = klp_find_object_symbol(obj->name,
> symname, &addr);
> + if (ret)
> + return ret;
> + sym->st_value = addr;
> + }
This is missing an important piece, I think. There's no way to
disambiguate duplicate symbols. Before, there was reloc->val, which
contained the symbol's address. There's nothing like that here.
I think we could use sym->st_value for that purpose. After Chris's
patch set gets merged, it could contain the sympos.
> }
> + ret = apply_relocate_add(info->sechdrs, info->strtab,
> + info->index.sym, relindex, pmod);
> }
>
> - return 0;
> + return ret;
> }
>
> static void notrace klp_ftrace_handler(unsigned long ip,
> @@ -741,12 +751,23 @@ static int klp_init_object_loaded(struct klp_patch
> *patch,
> struct klp_object *obj)
> {
> struct klp_func *func;
> + struct module *pmod;
> int ret;
>
> - if (obj->relocs) {
> - ret = klp_write_object_relocations(patch->mod, obj);
> + pmod = patch->mod;
> +
> + if (!list_empty(&obj->reloc_secs)) {
> + set_page_attributes(pmod->module_core,
> + pmod->module_core + pmod->core_text_size,
> + set_memory_rw);
> +
> + ret = klp_write_object_relocations(pmod, obj, patch);
> if (ret)
> return ret;
> +
> + set_page_attributes(pmod->module_core,
> + pmod->module_core + pmod->core_text_size,
> + set_memory_ro);
> }
>
> klp_for_each_func(obj, func) {
> --
> 2.4.3
>
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
