Re: [PATCH v4 2/3] livepatch: shuffle core.c function order

From: Petr Mladek
Date: Mon Oct 16 2017 - 10:42:21 EST


On Thu 2017-10-12 17:12:28, Jason Baron wrote:
> In preparation for __klp_enable_patch() to call a number of 'static'
> functions, in a subsequent patch, move them earlier in core.c. This patch
> should be a nop from a functional pov.
>
> Signed-off-by: Jason Baron <jbaron@xxxxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Cc: Jessica Yu <jeyu@xxxxxxxxxx>
> Cc: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: Miroslav Benes <mbenes@xxxxxxx>
> Cc: Petr Mladek <pmladek@xxxxxxxx>
> ---
> kernel/livepatch/core.c | 349 ++++++++++++++++++++++++------------------------
> 1 file changed, 173 insertions(+), 176 deletions(-)
>
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index b7f77be..f53eed5 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -283,6 +283,179 @@ static int klp_write_object_relocations(struct module *pmod,
> +static int klp_init_func(struct klp_object *obj, struct klp_func *func)
> +{
> + if (!func->old_name || !func->new_func)
> + return -EINVAL;
> +
> + INIT_LIST_HEAD(&func->stack_node);
> + func->patched = false;
> + func->transition = false;

You lost the change from the 1st patch:

list_add(&func->func_entry, &obj->func_list);

> +
> + /* The format for the sysfs directory is <function,sympos> where sympos
> + * is the nth occurrence of this symbol in kallsyms for the patched
> + * object. If the user selects 0 for old_sympos, then 1 will be used
> + * since a unique symbol will be the first occurrence.
> + */
> + return kobject_init_and_add(&func->kobj, &klp_ktype_func,
> + &obj->kobj, "%s,%lu", func->old_name,
> + func->old_sympos ? func->old_sympos : 1);
> +}

[...]

> +static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> +{
> + struct klp_func *func;
> + int ret;
> + const char *name;
> +
> + if (!obj->funcs)
> + return -EINVAL;
> +
> + obj->patched = false;
> + obj->mod = NULL;
> +
> + klp_find_object_module(obj);
> +
> + name = klp_is_module(obj) ? obj->name : "vmlinux";
> + ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object,
> + &patch->kobj, "%s", name);
> + if (ret)
> + return ret;
> +

Same here:

list_add(&obj->obj_entry, &patch->obj_list);
INIT_LIST_HEAD(&obj->func_list);
klp_for_each_func_static(obj, func) {

> + klp_for_each_func(obj, func) {
> + ret = klp_init_func(obj, func);
> + if (ret)
> + goto free;
> + }
> +
> + if (klp_is_object_loaded(obj)) {
> + ret = klp_init_object_loaded(patch, obj);
> + if (ret)
> + goto free;
> + }
> +
> + return 0;
> +
> +free:
> + klp_free_funcs_limited(obj, func);
> + kobject_put(&obj->kobj);
> + return ret;
> +}

Best Regards,
Petr