Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Sep 01, 2015 at 12:57:11PM +0200, Frederic Weisbecker wrote:

> > Agreed, kprobes also does it in generic code.
> 
> Well, the patchset got applied anyway and the reviews ignored...

Bugger, sorry about that, I meant to change it and things slipped, how
about I atone by doing the patch.

---
Subject: perf,hwbreakpoint,kprobe: Put kprobe test in generic code

Place the kprobe blacklist test for breakpoints in generic code; as in
general any place we should not kprobe we should not break on either, as
the reason for marking things nokprobe is that the code in question
cannot deal with interrupts of this kind.

Suggested-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/kernel/hw_breakpoint.c | 14 --------------
 include/linux/kprobes.h         |  7 ++++++-
 kernel/events/hw_breakpoint.c   |  8 ++++++++
 kernel/kprobes.c                |  4 ++--
 4 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 50a3fad5b89f..82b4a86b1e94 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -248,20 +248,6 @@ static int arch_build_bp_info(struct perf_event *bp)
 		info->type = X86_BREAKPOINT_RW;
 		break;
 	case HW_BREAKPOINT_X:
-		/*
-		 * We don't allow kernel breakpoints in places that are not
-		 * acceptable for kprobes.  On non-kprobes kernels, we don't
-		 * allow kernel breakpoints at all.
-		 */
-		if (bp->attr.bp_addr >= TASK_SIZE_MAX) {
-#ifdef CONFIG_KPROBES
-			if (within_kprobe_blacklist(bp->attr.bp_addr))
-				return -EINVAL;
-#else
-			return -EINVAL;
-#endif
-		}
-
 		info->type = X86_BREAKPOINT_EXECUTE;
 		/*
 		 * x86 inst breakpoints need to have a specific undefined len.
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 8f6849084248..54bb483fbfe4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,7 +267,7 @@ extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
 
-extern bool within_kprobe_blacklist(unsigned long addr);
+extern bool kprobe_blacklisted(unsigned long addr);
 
 struct kprobe_insn_cache {
 	struct mutex mutex;
@@ -391,6 +391,11 @@ void dump_kprobe(struct kprobe *kp);
 
 #else /* !CONFIG_KPROBES: */
 
+static inline bool kprobe_blacklisted(unsigned long addr)
+{
+	return false;
+}
+
 static inline int kprobes_built_in(void)
 {
 	return 0;
diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c
index 92ce5f4ccc26..7c08e6d1175c 100644
--- a/kernel/events/hw_breakpoint.c
+++ b/kernel/events/hw_breakpoint.c
@@ -384,6 +384,14 @@ static int validate_hw_breakpoint(struct perf_event *bp)
 		 */
 		if (!capable(CAP_SYS_ADMIN))
 			return -EPERM;
+
+		/*
+		 * We don't allow kernel breakpoints in places that are not
+		 * acceptable for kprobes.  On non-kprobes kernels, we don't
+		 * allow kernel breakpoints at all.
+		 */
+		if (kprobe_blacklisted(bp->attr.bp_addr))
+			return -EINVAL;
 	}
 
 	return 0;
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index d10ab6b9b5e0..adf646b17016 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1332,7 +1332,7 @@ bool __weak arch_within_kprobe_blacklist(unsigned long addr)
 	       addr < (unsigned long)__kprobes_text_end;
 }
 
-bool within_kprobe_blacklist(unsigned long addr)
+bool kprobe_blacklisted(unsigned long addr)
 {
 	struct kprobe_blacklist_entry *ent;
 
@@ -1442,7 +1442,7 @@ static int check_kprobe_address_safe(struct kprobe *p,
 
 	/* Ensure it is not in reserved area nor out of text */
 	if (!kernel_text_address((unsigned long) p->addr) ||
-	    within_kprobe_blacklist((unsigned long) p->addr) ||
+	    kprobe_blacklisted((unsigned long) p->addr) ||
 	    jump_label_text_reserved(p->addr, p->addr)) {
 		ret = -EINVAL;
 		goto out;