In cases of duplicate symbols, sympos will be used to disambiguate instead
of val. By default 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 in kallsyms for the patched object will be used
for patching if it is valid.
Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
as they are no longer used.
>From the klp_reloc structure remove val, as it can be refactored as a
local variable in klp_write_object_relocations.
Signed-off-by: Chris J Arges <[email protected]>
---
include/linux/livepatch.h | 5 ++-
kernel/livepatch/core.c | 81 +++++++++++++----------------------------------
2 files changed, 24 insertions(+), 62 deletions(-)
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 9808dd4..9149de4 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -68,8 +68,7 @@ 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)
+ * @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,7 +77,7 @@ struct klp_func {
*/
struct klp_reloc {
unsigned long loc;
- unsigned long val;
+ unsigned long sympos;
unsigned long type;
const char *name;
int addend;
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 4cd079c..071895f 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -202,45 +202,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;
-}
-
/*
* External symbols are exported symbols that are defined outside both
* the patched object and the patch.
@@ -266,6 +227,7 @@ static int klp_write_object_relocations(struct module *pmod,
struct klp_object *obj)
{
int ret;
+ unsigned long val;
struct klp_reloc *reloc;
if (WARN_ON(!klp_is_object_loaded(obj)))
@@ -275,29 +237,30 @@ 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;
- }
+ /* discover the address of the referenced symbol */
+ if (reloc->external) {
+ /*
+ * external symbols aren't specified by the user
+ * therefore sympos should not be used
+ */
+ if (reloc->sympos) {
+ pr_err("symbol '%s' is external and has sympos %lu\n",
+ reloc->name, reloc->sympos);
+ return -EINVAL;
+ }
+ ret = klp_find_external_symbol(pmod, reloc->name, &val);
+ } else
+ ret = klp_find_object_symbol(obj->mod->name,
+ reloc->name,
+ &val,
+ reloc->sympos);
+ if (ret)
+ return ret;
ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
- reloc->val + reloc->addend);
+ val + reloc->addend);
if (ret) {
pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
- reloc->name, reloc->val, ret);
+ reloc->name, val, ret);
return ret;
}
}
--
1.9.1
On Thu, Nov 12, 2015 at 10:59:52AM -0600, Chris J Arges wrote:
> In cases of duplicate symbols, sympos will be used to disambiguate instead
> of val. By default 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 in kallsyms for the patched object will be used
> for patching if it is valid.
>
> Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
> as they are no longer used.
>
> From the klp_reloc structure remove val, as it can be refactored as a
> local variable in klp_write_object_relocations.
>
> Signed-off-by: Chris J Arges <[email protected]>
> ---
> include/linux/livepatch.h | 5 ++-
> kernel/livepatch/core.c | 81 +++++++++++++----------------------------------
> 2 files changed, 24 insertions(+), 62 deletions(-)
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 9808dd4..9149de4 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -68,8 +68,7 @@ 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)
> + * @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,7 +77,7 @@ struct klp_func {
> */
> struct klp_reloc {
> unsigned long loc;
> - unsigned long val;
> + unsigned long sympos;
> unsigned long type;
> const char *name;
> int addend;
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 4cd079c..071895f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -202,45 +202,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;
> -}
> -
> /*
> * External symbols are exported symbols that are defined outside both
> * the patched object and the patch.
> @@ -266,6 +227,7 @@ static int klp_write_object_relocations(struct module *pmod,
> struct klp_object *obj)
> {
> int ret;
> + unsigned long val;
> struct klp_reloc *reloc;
>
> if (WARN_ON(!klp_is_object_loaded(obj)))
> @@ -275,29 +237,30 @@ 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;
> - }
> + /* discover the address of the referenced symbol */
> + if (reloc->external) {
> + /*
> + * external symbols aren't specified by the user
> + * therefore sympos should not be used
> + */
"external symbols' *objects* aren't specified by the user..."
> + if (reloc->sympos) {
> + pr_err("symbol '%s' is external and has sympos %lu\n",
> + reloc->name, reloc->sympos);
> + return -EINVAL;
> + }
> + ret = klp_find_external_symbol(pmod, reloc->name, &val);
> + } else
> + ret = klp_find_object_symbol(obj->mod->name,
> + reloc->name,
> + &val,
> + reloc->sympos);
> + if (ret)
> + return ret;
> ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc,
> - reloc->val + reloc->addend);
> + val + reloc->addend);
> if (ret) {
> pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n",
> - reloc->name, reloc->val, ret);
> + reloc->name, val, ret);
> return ret;
> }
> }
> --
> 1.9.1
>
--
Josh
On Thu 2015-11-12 10:59:52, Chris J Arges wrote:
> In cases of duplicate symbols, sympos will be used to disambiguate instead
> of val. By default 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 in kallsyms for the patched object will be used
> for patching if it is valid.
>
> Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
> as they are no longer used.
>
> >From the klp_reloc structure remove val, as it can be refactored as a
> local variable in klp_write_object_relocations.
>
> Signed-off-by: Chris J Arges <[email protected]>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 4cd079c..071895f 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -275,29 +237,30 @@ 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;
> - }
> + /* discover the address of the referenced symbol */
> + if (reloc->external) {
> + /*
> + * external symbols aren't specified by the user
> + * therefore sympos should not be used
> + */
> + if (reloc->sympos) {
> + pr_err("symbol '%s' is external and has sympos %lu\n",
> + reloc->name, reloc->sympos);
> + return -EINVAL;
> + }
To be precise, reloc->sympos == 1 is a valid value. Note that it will show
"symbol,1" in sysfs. I suggest the following to keep it simple:
if (reloc->sympos && (reloc->sympos != 1)) {
pr_err("relocated external symbols must be unique but '%s' has sympos %lu\n",
reloc->name, reloc->sympos);
return -EINVAL;
}
Then we could even get rid of the comment.
Huh, I am looking forward to get rid of the external symbols ;-)
Thank you,
Petr