Messages in this thread | | | Date | Thu, 12 Nov 2015 13:45:49 -0600 | From | Josh Poimboeuf <> | Subject | Re: [PATCH 1/4 v5] livepatch: add old_sympos as disambiguator field to klp_func |
| |
On Thu, Nov 12, 2015 at 10:59:50AM -0600, Chris J Arges wrote: > In cases of duplicate symbols, old_sympos will be used to disambiguate > instead of old_addr. 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 in kallsyms for the patched object > will be used for patching if it is valid. > > In addition, make old_addr an internal structure field not to be specified > by the user. Finally, remove klp_find_verify_func_addr as it can be > replaced by klp_find_object_symbol directly. > > Signed-off-by: Chris J Arges <chris.j.arges@canonical.com> > --- > include/linux/livepatch.h | 20 ++++++++------- > kernel/livepatch/core.c | 65 ++++++++++++++++++++--------------------------- > 2 files changed, 38 insertions(+), 47 deletions(-) > > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h > index 31db7a0..3d18dff 100644 > --- a/include/linux/livepatch.h > +++ b/include/linux/livepatch.h > @@ -37,8 +37,9 @@ enum klp_state { > * struct klp_func - function structure for live patching > * @old_name: name of the function to be patched > * @new_func: pointer to the patched function code > - * @old_addr: a hint conveying at what address the old function > - * can be found (optional, vmlinux patches only) > + * @old_sympos: a hint indicating which symbol position the old function > + * can be found (optional) > + * @old_addr: the address of the function being patched > * @kobj: kobject for sysfs resources > * @state: tracks function-level patch application state > * @stack_node: list node for klp_ops func_stack list > @@ -47,17 +48,18 @@ struct klp_func { > /* external */ > const char *old_name; > void *new_func; > + > /* > - * The old_addr field is optional and can be used to resolve > - * duplicate symbol names in the vmlinux object. If this > - * information is not present, the symbol is located by name > - * with kallsyms. If the name is not unique and old_addr is > - * not provided, the patch application fails as there is no > - * way to resolve the ambiguity. > + * The old_sympos field is optional and can be used to resolve > + * duplicate symbol names in livepatch objects. If this field is zero, > + * it is expected the symbol is unique, otherwise patching fails. If > + * this value is greater than zero then that occurrence of the symbol > + * in kallsyms for the given object is used. > */ > - unsigned long old_addr; > + unsigned long old_sympos; > > /* internal */ > + unsigned long old_addr; > struct kobject kobj; > enum klp_state state; > struct list_head stack_node; > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 6e53441..479d75e 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -135,13 +135,8 @@ struct klp_find_arg { > const char *objname; > const char *name; > unsigned long addr; > - /* > - * If count == 0, the symbol was not found. If count == 1, a unique > - * match was found and addr is set. If count > 1, there is > - * unresolvable ambiguity among "count" number of symbols with the same > - * name in the same object. > - */ > unsigned long count; > + unsigned long pos; > }; > > static int klp_find_callback(void *data, const char *name, > @@ -159,36 +154,46 @@ static int klp_find_callback(void *data, const char *name, > return 0; > > /* > - * args->addr might be overwritten if another match is found > - * but klp_find_object_symbol() handles this and only returns the > - * addr if count == 1. > + * Increment and assign the address. Return 1 when the desired symbol > + * position is found or the search for a unique symbol fails. > */ > args->addr = addr; > args->count++; > + if (((args->pos > 0) && (args->count == args->pos)) || > + ((args->pos == 0) && (args->count > 1))) > + return 1; > > return 0; > } > > static int klp_find_object_symbol(const char *objname, const char *name, > - unsigned long *addr) > + unsigned long *addr, unsigned long sympos)
One nit about this interface. I think it would be clearer if the sympos argument came before the addr argument:
static int klp_find_object_symbol(const char *objname, const char *name, unsigned long sympos, unsigned long addr)
Because:
1. it puts all input arguments before the output argument; and
2. it puts the sympos in its logical place immediately after the function name.
-- Josh
| |