linux-kernel - Re: livepatch: reuse module loader code to write relocations

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160113183924.GA980@packer-debian-8-amd64.digitalocean.com>
Date:	Wed, 13 Jan 2016 13:39:25 -0500
From:	Jessica Yu <jeyu@...hat.com>
To:	Miroslav Benes <mbenes@...e.cz>
Cc:	Rusty Russell <rusty@...tcorp.com.au>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Seth Jennings <sjenning@...hat.com>,
	Jiri Kosina <jikos@...nel.org>,
	Vojtech Pavlik <vojtech@...e.com>,
	Jonathan Corbet <corbet@....net>, linux-api@...r.kernel.org,
	live-patching@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
	linux-doc@...r.kernel.org
Subject: Re: livepatch: reuse module loader code to write relocations

+++ Miroslav Benes [13/01/16 10:19 +0100]:
>On Fri, 8 Jan 2016, Jessica Yu wrote:
>
>>  static int klp_write_object_relocations(struct module *pmod,
>>  					struct klp_object *obj)
>>  {
>> -	int ret = 0;
>> -	unsigned long val;
>> -	struct klp_reloc *reloc;
>> +	int i, len, ret = 0;
>> +	char *secname;
>> +	const char *objname;
>>
>>  	if (WARN_ON(!klp_is_object_loaded(obj)))
>>  		return -EINVAL;
>>
>> -	if (WARN_ON(!obj->relocs))
>> -		return -EINVAL;
>> +	objname = klp_is_module(obj) ? obj->name : "vmlinux";
>>
>>  	module_disable_ro(pmod);
>> +	/* For each klp rela section for this object */
>> +	for (i = 1; i < pmod->info->hdr->e_shnum; i++) {
>> +		if (!(pmod->info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH))
>> +			continue;
>
>One more thing. If the module does not specify it is a live patch module
>in modinfo (with MODULE_INFO(livepatch, "Y")), but it is a perfect live
>patch module otherwise (it calls klp_register_patch in its init function),
>the kernel crashes here. pmod->info is not initialized at all. This should
>be fixed. Perhaps the easiest would be to call
>klp_write_object_relocations() in klp_init_object_loaded() only if
>is_livepatch_module() returns true. Similar to a check for obj->relocs
>before.

Hm yes, that's a problem. To remedy this, I think it makes sense to
require all livepatch modules to identify themselves with the modinfo
attribute, since it is a very simple requirement. If some module calls
klp_register_patch() and it does not have the livepatch attribute,
klp_register_patch() can just return an error. We can call
is_livepatch_module() at the beginning of klp_register_patch(), and
proceed only if the check succeeds, since we'll then know that the
required structures have been properly initialized in the module
loader. What do you think?

Jessica