Messages in this thread Patch in this message | | | Date | Fri, 13 Nov 2015 17:27:05 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH 1/4 v5] livepatch: add old_sympos as disambiguator field to klp_func |
| |
On Thu 2015-11-12 10:59:50, 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> > --- > 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 > @@ -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++;
I would avoid the obvious things and move the comment here:
/* * Finish the search when the symbol is found on the desired * position or the position is not defined for non-unique * symbol. */
> + if (((args->pos > 0) && (args->count == args->pos)) || > + ((args->pos == 0) && (args->count > 1))) > + return 1;
I wonder if this is better readable:
if ((args->pos && (args->count == args->pos)) || (!args->pos && (args->count > 1))) return 1;
> return 0; > } > [...] > @@ -277,7 +261,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);
Please, add a comment here that external symbols always have to be unique.
> } > > static int klp_write_object_relocations(struct module *pmod, > @@ -307,7 +291,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);
Please, mention in the commit message that the support for sympos will be added into relocations in a separate patch. Or add a FIXME comment here. It will make easier the review and git history archaeology.
Thank you, Petr
| |