| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 11 Nov 2015 11:57:31 -0600
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Chris J Arges <chris.j.arges@...onical.com>
Cc: live-patching@...r.kernel.org, jeyu@...hat.com,
Seth Jennings <sjenning@...hat.com>,
Jiri Kosina <jikos@...nel.org>,
Vojtech Pavlik <vojtech@...e.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3 v4] livepatch: add old_sympos as disambiguator field
to klp_reloc
On Wed, Nov 11, 2015 at 10:29:00AM -0600, Chris J Arges wrote:
> In cases of duplicate symbols, sympos will be used to disambiguate instead
> of val. By default old_sympos will be 0, and patching will only succeed if
> the symbol is unique. Specifying a positive value will ensure that
> occurrence of the symbol will be used for patching if it is valid.
>
> Signed-off-by: Chris J Arges <chris.j.arges@...onical.com>
> ---
> include/linux/livepatch.h | 5 ++--
> kernel/livepatch/core.c | 74 ++++++++++-------------------------------------
> 2 files changed, 18 insertions(+), 61 deletions(-)
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index df7b752..fb968a2 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -68,8 +68,8 @@ struct klp_func {
> /**
> * struct klp_reloc - relocation structure for live patching
> * @loc: address where the relocation will be written
> - * @val: address of the referenced symbol (optional,
> - * vmlinux patches only)
> + * @val: address of the referenced symbol
> + * @sympos: position in kallsyms to disambiguate symbols (optional)
> * @type: ELF relocation type
> * @name: name of the referenced symbol (for lookup/verification)
> * @addend: offset from the referenced symbol
> @@ -78,6 +78,7 @@ struct klp_func {
> struct klp_reloc {
> unsigned long loc;
> unsigned long val;
> + unsigned long sympos;
> unsigned long type;
> const char *name;
> int addend;
I think 'val' can be removed from this struct, since it's now basically
just a private variable of klp_write_object_relocations().
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 26f9778..4eb8691 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -207,45 +207,6 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> return -EINVAL;
> }
>
> -struct klp_verify_args {
> - const char *name;
> - const unsigned long addr;
> -};
> -
> -static int klp_verify_callback(void *data, const char *name,
> - struct module *mod, unsigned long addr)
> -{
> - struct klp_verify_args *args = data;
> -
> - if (!mod &&
> - !strcmp(args->name, name) &&
> - args->addr == addr)
> - return 1;
> -
> - return 0;
> -}
> -
> -static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr)
> -{
> - struct klp_verify_args args = {
> - .name = name,
> - .addr = addr,
> - };
> - int ret;
> -
> - mutex_lock(&module_mutex);
> - ret = kallsyms_on_each_symbol(klp_verify_callback, &args);
> - mutex_unlock(&module_mutex);
> -
> - if (!ret) {
> - pr_err("symbol '%s' not found at specified address 0x%016lx, kernel mismatch?\n",
> - name, addr);
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> static int klp_find_verify_func_addr(struct klp_object *obj,
> struct klp_func *func)
> {
> @@ -261,7 +222,7 @@ static int klp_find_verify_func_addr(struct klp_object *obj,
> * object is either vmlinux or the kmod being patched).
> */
> static int klp_find_external_symbol(struct module *pmod, const char *name,
> - unsigned long *addr)
> + unsigned long *addr, unsigned long sympos)
> {
> const struct kernel_symbol *sym;
>
For "external" symbols, the object isn't specified by the user, and
since sympos is per-object, the value of sympos is undefined. Instead
I think it should always pass 0 to klp_find_object_symbol() below.
In line with that, since reloc->external and reloc->sympos don't mix,
maybe klp_write_object_relocations() should return -EINVAL if external
is set and sympos is non-zero.
> @@ -276,7 +237,7 @@ static int klp_find_external_symbol(struct module *pmod, const char *name,
> preempt_enable();
>
> /* otherwise check if it's in another .o within the patch module */
> - return klp_find_object_symbol(pmod->name, name, addr, 0);
> + return klp_find_object_symbol(pmod->name, name, addr, sympos);
> }
>
> static int klp_write_object_relocations(struct module *pmod,
> @@ -292,24 +253,19 @@ static int klp_write_object_relocations(struct module *pmod,
> 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, 0);
> - if (ret)
> - return ret;
> - }
> + /* reloc->val needs to be discovered */
> + if (reloc->external)
> + ret = klp_find_external_symbol(pmod,
> + reloc->name,
> + &reloc->val,
> + reloc->sympos);
> + else
> + ret = klp_find_object_symbol(obj->mod->name,
> + reloc->name,
> + &reloc->val,
> + reloc->sympos);
> + if (ret)
> + return ret;
> ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> reloc->val + reloc->addend);
> if (ret) {
> --
> 1.9.1
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists