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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 16 Oct 2017 10:45:14 -0400
From:   Jason Baron <jbaron@...mai.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
        jpoimboe@...hat.com, jeyu@...nel.org, jikos@...nel.org,
        mbenes@...e.cz
Subject: Re: [PATCH v4 2/3] livepatch: shuffle core.c function order



On 10/16/2017 10:42 AM, Petr Mladek wrote:
> 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@...mai.com>
>> Cc: Josh Poimboeuf <jpoimboe@...hat.com>
>> Cc: Jessica Yu <jeyu@...nel.org>
>> Cc: Jiri Kosina <jikos@...nel.org>
>> Cc: Miroslav Benes <mbenes@...e.cz>
>> Cc: Petr Mladek <pmladek@...e.com>
>> ---
>>  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) {

Yes, thanks. I noticed that too when insertion didn't match deletion :(
The final patch ends up adding it back, instead of this one, so the
states of things after patch 3 is ok.

Thanks,

-Jason