On Tue 2015-11-03 11:52:08, Miroslav Benes wrote: > On Mon, 2 Nov 2015, Chris J Arges wrote: > > [...] > > > +static int klp_get_func_pos_callback(void *data, const char *name, > > + struct module *mod, unsigned long addr) > > +{ > > + struct klp_find_arg *args = data; > > + > > + if ((mod && !args->objname) || (!mod && args->objname)) > > + return 0; > > + > > + if (strcmp(args->name, name)) > > + return 0; > > + > > + if (args->objname && strcmp(args->objname, mod->name)) > > + return 0; > > + > > + /* on address match, return 1 to break kallsyms_on_each_symbol loop */ > > + if (args->addr == addr) > > + return 1; > > + > > + /* if we don't match addr, count instance of named symbol */ > > + args->count++; > > + > > + return 0; > > +} > > + > > +static int klp_get_func_pos(struct klp_object *obj, struct klp_func *func) > > +{ > > + struct klp_find_arg args = { > > + .objname = obj->name, > > + .name = func->old_name, > > + .addr = func->old_addr, > > + .count = 0, > > + }; > > + > > + mutex_lock(&module_mutex); > > + kallsyms_on_each_symbol(klp_get_func_pos_callback, &args); > > + mutex_unlock(&module_mutex); > > + > > + return args.count; > > +} > > + > > static int klp_init_func(struct klp_object *obj, struct klp_func *func) > > { > > INIT_LIST_HEAD(&func->stack_node); > > func->state = KLP_DISABLED; > > > > return kobject_init_and_add(&func->kobj, &klp_ktype_func, > > - &obj->kobj, "%s", func->old_name); > > + &obj->kobj, "%s,%d", func->old_name, > > + klp_get_func_pos(obj, func)); > > } > > There is a problem which I missed before. klp_init_func() is called before > klp_find_verify_func_addr() in klp_init_object(). This means that > func->old_addr is either not verified yet or worse it is still 0. This > means that klp_get_func_pos_callback() never returns 1 and is thus called > on each symbol. So if you for example patched cmdline_proc_show the > resulting directory in sysfs would be called cmdline_proc_show,1 because > addr is never matched. Had old_addr been specified the name would have > been probably correct, but not for sure. This might happen when the function name is unique. Then we might but we do not need to pre-define the address in the patch. Also I would omit the suffix at all when it is the first occurrence. It will cause that unique symbols will not be numbered. Best Regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html