On Wed, Nov 11, 2015 at 10:28:59AM -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 will be used for patching if it is
> valid. Finally, old_addr is now an internal structure element and not to
> be specified by the user.
>
> Signed-off-by: Chris J Arges <[email protected]>
> ---
> include/linux/livepatch.h | 20 ++++++++++--------
> kernel/livepatch/core.c | 53
> +++++++++++++++++++++++------------------------
> 2 files changed, 37 insertions(+), 36 deletions(-)
>
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 31db7a0..df7b752 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 is used.
I would clarify this:
...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..26f9778 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -142,6 +142,7 @@ struct klp_find_arg {
> * name in the same object.
> */
> unsigned long count;
> + unsigned long pos;
There's a comment above this that says:
/*
* 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.
*/
That comment is no longer accurate and can probably be removed since IMO
the purpose of 'count' is obvious.
> };
>
> static int klp_find_callback(void *data, const char *name,
> @@ -159,36 +160,45 @@ 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 address, return only if checking pos and
> + * it matches count.
> */
> - args->addr = addr;
> args->count++;
> + args->addr = addr;
> + if ((args->pos > 0) && (args->count == args->pos))
> + 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)
> {
> struct klp_find_arg args = {
> .objname = objname,
> .name = name,
> .addr = 0,
> - .count = 0
> + .count = 0,
> + .pos = sympos,
> };
>
> mutex_lock(&module_mutex);
> kallsyms_on_each_symbol(klp_find_callback, &args);
> mutex_unlock(&module_mutex);
>
> - if (args.count == 0)
> + /*
> + * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> + * otherwise ensure the symbol position count matches sympos.
> + */
> + if (args.addr == 0)
> pr_err("symbol '%s' not found in symbol table\n", name);
> - else if (args.count > 1)
> + else if (args.count > 1 && sympos == 0) {
> pr_err("unresolvable ambiguity (%lu matches) on symbol '%s' in
> object '%s'\n",
> args.count, name, objname);
> - else {
> + } else if (sympos != args.count && sympos > 0) {
> + pr_err("symbol position %lu for symbol '%s' in object '%s' not
> found\n",
> + sympos, name, objname ? objname : "vmlinux");
> + } else {
> *addr = args.addr;
> return 0;
> }
> @@ -239,22 +249,11 @@ static int klp_verify_vmlinux_symbol(const char *name,
> unsigned long addr)
> static int klp_find_verify_func_addr(struct klp_object *obj,
> struct klp_func *func)
> {
> - int ret;
> -
> -#if defined(CONFIG_RANDOMIZE_BASE)
> - /* If KASLR has been enabled, adjust old_addr accordingly */
> - if (kaslr_enabled() && func->old_addr)
> - func->old_addr += kaslr_offset();
> -#endif
> -
> - if (!func->old_addr || klp_is_module(obj))
> - ret = klp_find_object_symbol(obj->name, func->old_name,
> - &func->old_addr);
> - else
> - ret = klp_verify_vmlinux_symbol(func->old_name,
> - func->old_addr);
> -
> - return ret;
> + /*
> + * Verify the symbol, find old_addr, and write it to the structure.
> + */
> + return klp_find_object_symbol(obj->name, func->old_name,
> + &func->old_addr, func->old_sympos);
klp_find_verify_func_addr() is no longer correctly named and can
probably be removed since klp_init_object_loaded() can call
klp_find_object_symbol() directly.
> }
>
> /*
> @@ -277,7 +276,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);
> + return klp_find_object_symbol(pmod->name, name, addr, 0);
> }
>
> static int klp_write_object_relocations(struct module *pmod,
> @@ -307,7 +306,7 @@ static int klp_write_object_relocations(struct module
> *pmod,
> else
> ret = klp_find_object_symbol(obj->mod->name,
> reloc->name,
> - &reloc->val);
> + &reloc->val, 0);
> if (ret)
> return ret;
> }
> --
> 1.9.1
>
--
Josh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
