linux-kernel - [PATCH 4/4] kprobes, x86/alternatives: preempt_disable() when check smp_alt

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]

Message-ID: <1509195057-117316-4-git-send-email-zhouchengming1@huawei.com>
Date:   Sat, 28 Oct 2017 20:50:57 +0800
From:   Zhou Chengming <zhouchengming1@...wei.com>
To:     <mhiramat@...nel.org>, <bp@...e.de>, <peterz@...radead.org>,
        <ananth@...ux.vnet.ibm.com>, <anil.s.keshavamurthy@...el.com>,
        <davem@...emloft.net>, <tglx@...utronix.de>, <jkosina@...e.cz>,
        <rostedt@...dmis.org>, <mjurczyk@...gle.com>
CC:     <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        <zhouchengming1@...wei.com>
Subject: [PATCH 4/4] kprobes, x86/alternatives: preempt_disable() when check smp_alt_modules

Fixes: 2cfa197 "ftrace/alternatives: Introducing *_text_reserved
functions"

We use alternatives_text_reserved() to check if the address is in
the fixed pieces of alternative reserved, but the problem is that
we don't hold the smp_alt mutex when call this function. So the list
traversal may encounter a deleted list_head if another path is doing
alternatives_smp_module_del().

One solution is that we can hold smp_alt mutex before call this
function, but the difficult point is that the callers of this
functions, arch_prepare_kprobe() and arch_prepare_optimized_kprobe(),
are called inside the text_mutex. So we must hold smp_alt mutex
before we go into these arch dependent code. But we can't now,
the smp_alt mutex is the arch dependent part, only x86 has it.
Maybe we can export another arch dependent callback to solve this.

Another simpler solution reuse the text_mutex to protect smp_alt_modules
list, all the arch dependent checks of kprobes are inside the
text_mutex, so it's safe. But it will use one text_mutex to protect
2 resources, which is not a good idea. And the original patch and
related discussions are here:
https://patchwork.kernel.org/patch/10029465/

This patchset take a different way to solve the problem. We could
remove the smp_alt mutex, only use preempt_disable() to protect
the smp_alt_modules list, since the list only be used when UP now.
So alternatives_text_reserved() also only need to preempt_disable()
when do the smp_alt_modules list check, it's safe now.

Signed-off-by: Zhou Chengming <zhouchengming1@...wei.com>
---
 arch/x86/kernel/alternative.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7eab6f6..b278cad 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -575,6 +575,9 @@ int alternatives_text_reserved(void *start, void *end)
 	const s32 *poff;
 	u8 *text_start = start;
 	u8 *text_end = end;
+	int ret = 0;
+
+	preempt_disable();

 	list_for_each_entry(mod, &smp_alt_modules, next) {
 		if (mod->text > text_end || mod->text_end < text_start)
@@ -582,12 +585,16 @@ int alternatives_text_reserved(void *start, void *end)
 		for (poff = mod->locks; poff < mod->locks_end; poff++) {
 			const u8 *ptr = (const u8 *)poff + *poff;

-			if (text_start <= ptr && text_end > ptr)
-				return 1;
+			if (text_start <= ptr && text_end > ptr) {
+				ret = 1;
+				goto out;
+			}
 		}
 	}

-	return 0;
+out:
+	preempt_enable();
+	return ret;
 }
 #endif /* CONFIG_SMP */

-- 
1.8.3.1