From patchwork Tue Jul 12 12:40:02 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tixy X-Patchwork-Id: 968282 Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by demeter1.kernel.org (8.14.4/8.14.4) with ESMTP id p6CCeOTY010650 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 12 Jul 2011 12:40:46 GMT Received: from canuck.infradead.org ([2001:4978:20e::1]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1QgcG4-0001Tk-0x; Tue, 12 Jul 2011 12:40:12 +0000 Received: from localhost ([127.0.0.1] helo=canuck.infradead.org) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QgcG3-0005iZ-L7; Tue, 12 Jul 2011 12:40:11 +0000 Received: from smarthost03.mail.zen.net.uk ([212.23.3.142]) by canuck.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1QgcG0-0005iG-3F for linux-arm-kernel@lists.infradead.org; Tue, 12 Jul 2011 12:40:09 +0000 Received: from [82.69.122.217] (helo=plug1) by smarthost03.mail.zen.net.uk with esmtpsa (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1QgcFy-0000c9-18 for linux-arm-kernel@lists.infradead.org; Tue, 12 Jul 2011 12:40:06 +0000 Received: from [192.168.2.20] (helo=computer2) by plug1 with esmtp (Exim 4.72) (envelope-from ) id 1QgcFx-0005sb-In for linux-arm-kernel@lists.infradead.org; Tue, 12 Jul 2011 13:40:05 +0100 Received: from tixy by computer2 with local (Exim 4.72) (envelope-from ) id 1QgcFx-0001T3-GK for linux-arm-kernel@lists.infradead.org; Tue, 12 Jul 2011 13:40:05 +0100 From: Tixy To: linux-arm-kernel@lists.infradead.org Subject: [PATCH] [RFC PATCH] ARM: kprobes: Make breakpoint setting/clearing SMP safe Date: Tue, 12 Jul 2011 13:40:02 +0100 Message-Id: <1310474402-5398-1-git-send-email-tixy@yxit.co.uk> X-Mailer: git-send-email 1.7.2.5 X-Originating-Smarthost03-IP: [82.69.122.217] X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20110712_084008_383340_95D4CACF X-CRM114-Status: GOOD ( 21.86 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.3.1 on canuck.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [212.23.3.142 listed in list.dnswl.org] X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Tue, 12 Jul 2011 12:40:46 +0000 (UTC) From: Jon Medhurst On SMP systems, kprobes has two different issues when manipulating breakpoints. When disarming probes, with two CPUs (A and B) we can get: - A takes undef exception due to hitting a probe breakpoint. - B disarms probe, replacing the breakpoint with original instruction. - A's undef handler reads instruction at PC, doesn't see kprobe breakpoint, therefore doesn't call kprobe_handler and the process takes a fatal exception. The second issue occurs when both arming and disarming probes on a 32-bit Thumb instruction which straddles two memory words. In this case it's possible that another CPU will read 16 bits from the new instruction and 16 from the old. Both these issues are known and the kprobe implementation uses stop_machine() to avoid them, however, this is not sufficient. stop_machine() does not perform any kind on synchronisation between CPUs so it it still possible for one CPU to call the breakpoint changing function before another CPU has been interrupted to do likewise. To fix this problem, this patch creates a new function sync_stop_machine() which ensures that all online CPUs execute the specified function at the same time. I am looking for feedback: - Have I got my understanding of stop_machine() right? - Are there other APIs to do what we need? - Is my patch robust. - Do I have the correct barriers? Signed-off-by: Jon Medhurst --- arch/arm/kernel/kprobes.c | 55 +++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 53 insertions(+), 2 deletions(-) diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c index 129c116..c6b7f2a 100644 --- a/arch/arm/kernel/kprobes.c +++ b/arch/arm/kernel/kprobes.c @@ -103,6 +103,57 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) return 0; } +struct sync_stop_machine_data { + atomic_t begin; + atomic_t end; + int (*fn)(void *); + void *data; +}; + +static int __kprobes __sync_stop_machine(void *data) +{ + struct sync_stop_machine_data *ssmd; + int ret; + + ssmd = (struct sync_stop_machine_data *)data; + + /* Wait until all CPUs are ready before calling the function */ + atomic_dec(&ssmd->begin); + smp_mb__after_atomic_dec(); + while (atomic_read(&ssmd->begin)) {} + + ret = ssmd->fn(ssmd->data); + + /* Wait for all CPUs to finish 'fn' before returning */ + smp_mb__before_atomic_dec(); + atomic_dec(&ssmd->end); + while (atomic_read(&ssmd->end)) {} + + return ret; +} + +static int __kprobes sync_stop_machine(int (*fn)(void *), void *data) +{ + struct sync_stop_machine_data ssmd = { + .fn = fn, + .data = data + }; + int num_cpus; + int ret; + + get_online_cpus(); /* So number of CPUs can't change */ + + num_cpus = num_online_cpus(); + atomic_set(&ssmd.begin, num_cpus); + atomic_set(&ssmd.end, num_cpus); + smp_wmb(); + + ret = stop_machine(__sync_stop_machine, &ssmd, &cpu_online_map); + + put_online_cpus(); + return ret; +} + #ifdef CONFIG_THUMB2_KERNEL /* @@ -127,7 +178,7 @@ void __kprobes arch_arm_kprobe(struct kprobe *p) flush_insns(addr, sizeof(u16)); } else if (addr & 2) { /* A 32-bit instruction spanning two words needs special care */ - stop_machine(set_t32_breakpoint, (void *)addr, &cpu_online_map); + sync_stop_machine(set_t32_breakpoint, (void *)addr); } else { /* Word aligned 32-bit instruction can be written atomically */ u32 bkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION; @@ -190,7 +241,7 @@ int __kprobes __arch_disarm_kprobe(void *p) void __kprobes arch_disarm_kprobe(struct kprobe *p) { - stop_machine(__arch_disarm_kprobe, p, &cpu_online_map); + sync_stop_machine(__arch_disarm_kprobe, p); } void __kprobes arch_remove_kprobe(struct kprobe *p)