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@xxxxxxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
