linux-kernel - Re: Bug with paravirt ops and livepatches

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:	Fri, 1 Apr 2016 21:35:34 +0200 (CEST)
From:	Jiri Kosina <jikos@...nel.org>
To:	Chris J Arges <chris.j.arges@...onical.com>
cc:	Miroslav Benes <mbenes@...e.cz>, jeyu@...hat.com,
	jpoimboe@...hat.com, eugene.shatokhin@...alab.ru,
	live-patching@...r.kernel.org,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	pmladek@...e.cz
Subject: Re: Bug with paravirt ops and livepatches

On Fri, 1 Apr 2016, Chris J Arges wrote:

> Loading, please wait...
> starting version 229
> [    1.182869] random: udevadm urandom read with 2 bits of entropy available
> [    1.241404] BUG: unable to handle kernel paging request at ffffffffc000f35f

Gah, we surely can't change pages with RO PTE. Thanks for such a prompt 
testing. You do have CONFIG_DEBUG_SET_MODULE_RONX set, don't you?

The patch below should fix that by marking the module RO (and relevant 
parts NX) only when it's guaranteed that .text is not going to be modified 
any more (and includes the error handling fix Miroslav spotted as well).

Thanks.



diff --git a/kernel/module.c b/kernel/module.c
index 5f71aa6..430606d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3211,7 +3211,7 @@ int __weak module_finalize(const Elf_Ehdr *hdr,
 	return 0;
 }
 
-static int post_relocation(struct module *mod, const struct load_info *info)
+static void post_relocation(struct module *mod, const struct load_info *info)
 {
 	/* Sort exception table now relocations are done. */
 	sort_extable(mod->extable, mod->extable + mod->num_exentries);
@@ -3222,9 +3222,6 @@ static int post_relocation(struct module *mod, const struct load_info *info)
 
 	/* Setup kallsyms-specific fields. */
 	add_kallsyms(mod, info);
-
-	/* Arch-specific module finalizing. */
-	return module_finalize(info->hdr, info->sechdrs, mod);
 }
 
 /* Is this module of this name done loading?  No locks held. */
@@ -3441,10 +3438,6 @@ static int complete_formation(struct module *mod, struct load_info *info)
 	/* This relies on module_mutex for list integrity. */
 	module_bug_finalize(info->hdr, info->sechdrs, mod);
 
-	/* Set RO and NX regions */
-	module_enable_ro(mod);
-	module_enable_nx(mod);
-
 	/* Mark state as coming so strong_try_module_get() ignores us,
 	 * but kallsyms etc. can see us. */
 	mod->state = MODULE_STATE_COMING;
@@ -3562,9 +3555,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err < 0)
 		goto free_modinfo;
 
-	err = post_relocation(mod, info);
-	if (err < 0)
-		goto free_modinfo;
+	post_relocation(mod, info);
 
 	flush_module_icache(mod);
 
@@ -3589,6 +3580,15 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err)
 		goto bug_cleanup;
 
+	/* Arch-specific module finalizing. */
+	err = module_finalize(info->hdr, info->sechdrs, mod);
+	if (err)
+		goto coming_cleanup;
+
+	/* Set RO and NX regions */
+	module_enable_ro(mod);
+	module_enable_nx(mod);
+
 	/* Module is ready to execute: parsing args may do that. */
 	after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp,
 				  -32768, 32767, mod,

-- 
Jiri Kosina
SUSE Labs