Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1196567 > unrolled thread

[PATCH 0/3] perf: hw_breakpoint safety improvements

Started byAndy Lutomirski <luto@kernel.org>
First post2015-07-31 05:40 +0200
Last post2015-08-05 01:30 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/3] perf: hw_breakpoint safety improvements Andy Lutomirski <luto@kernel.org> - 2015-07-31 05:40 +0200
    [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints Andy Lutomirski <luto@kernel.org> - 2015-07-31 05:40 +0200
      [tip:perf/core] perf/x86/hw_breakpoints:   Fix check for kernel-space breakpoints tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2015-08-04 11:00 +0200
      Re: [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace  breakpoints Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-04 18:20 +0200
    [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe Andy Lutomirski <luto@kernel.org> - 2015-07-31 05:40 +0200
      [tip:perf/core] perf/x86/hw_breakpoints:   Disallow kernel breakpoints unless kprobe-safe tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2015-08-04 11:00 +0200
      Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints  unless kprobe-safe Frederic Weisbecker <fweisbec@gmail.com> - 2015-08-04 18:00 +0200
        RE: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints  unless kprobe-safe 平松雅巳 / HIRAMATU,MASAMI   <masami.hiramatsu.pt@hitachi.com> - 2015-08-05 01:30 +0200

#1196567 — [PATCH 0/3] perf: hw_breakpoint safety improvements

FromAndy Lutomirski <luto@kernel.org>
Date2015-07-31 05:40 +0200
Subject[PATCH 0/3] perf: hw_breakpoint safety improvements
Message-ID<pS9kR-1Uh-3@gated-at.bofh.it>
Hi, Peter-

Here are some baby steps toward eliminating nested NMIs.  What do
you think?

Andy Lutomirski (3):
  x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe
  x86/perf/hw_breakpoint: Improve range breakpoint validation
  x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints

 arch/x86/kernel/hw_breakpoint.c | 31 ++++++++++++++++++++++++++++++-
 include/linux/kprobes.h         |  2 ++
 kernel/kprobes.c                |  2 +-
 3 files changed, 33 insertions(+), 2 deletions(-)

-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1196568 — [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints

FromAndy Lutomirski <luto@kernel.org>
Date2015-07-31 05:40 +0200
Subject[PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints
Message-ID<pS9kS-1Uh-7@gated-at.bofh.it>
In reply to#1196567
The check looked wrong, although I think it was actually safe.  TASK_SIZE
is unnecessarily small for compat tasks, and it wasn't possible to make
a range breakpoint so large it started in user space and ended in kernel
space.

Nonetheless, let's fix up the check for the benefit of future
readers.  A breakpoint is in the kernel if either end is in the
kernel.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/hw_breakpoint.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 6f345d302cf6..50a3fad5b89f 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -180,7 +180,11 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
 	va = info->address;
 	len = bp->attr.bp_len;
 
-	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);
+	/*
+	 * We don't need to worry about va + len - 1 overflowing:
+	 * we already require that va is aligned to a multiple of len.
+	 */
+	return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
 }
 
 int arch_bp_generic_fields(int x86_len, int x86_type,
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1199555 — [tip:perf/core] perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints

Fromtip-bot for Andy Lutomirski <tipbot@zytor.com>
Date2015-08-04 11:00 +0200
Subject[tip:perf/core] perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints
Message-ID<pTGeL-5Af-35@gated-at.bofh.it>
In reply to#1196568
Commit-ID:  27747f8bc355a2808ca9e490ab6866acd85b4c16
Gitweb:     http://git.kernel.org/tip/27747f8bc355a2808ca9e490ab6866acd85b4c16
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 30 Jul 2015 20:32:42 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 4 Aug 2015 10:16:55 +0200

perf/x86/hw_breakpoints: Fix check for kernel-space breakpoints

The check looked wrong, although I think it was actually safe.  TASK_SIZE
is unnecessarily small for compat tasks, and it wasn't possible to make
a range breakpoint so large it started in user space and ended in kernel
space.

Nonetheless, let's fix up the check for the benefit of future
readers.  A breakpoint is in the kernel if either end is in the
kernel.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/136be387950e78f18cea60e9d1bef74465d0ee8f.1438312874.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/hw_breakpoint.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 6f345d3..50a3fad 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -180,7 +180,11 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
 	va = info->address;
 	len = bp->attr.bp_len;
 
-	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);
+	/*
+	 * We don't need to worry about va + len - 1 overflowing:
+	 * we already require that va is aligned to a multiple of len.
+	 */
+	return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
 }
 
 int arch_bp_generic_fields(int x86_len, int x86_type,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1200140 — Re: [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-08-04 18:20 +0200
SubjectRe: [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints
Message-ID<pTN6y-7fJ-21@gated-at.bofh.it>
In reply to#1196568
On Thu, Jul 30, 2015 at 08:32:42PM -0700, Andy Lutomirski wrote:
> The check looked wrong, although I think it was actually safe.  TASK_SIZE
> is unnecessarily small for compat tasks, and it wasn't possible to make
> a range breakpoint so large it started in user space and ended in kernel
> space.
> 
> Nonetheless, let's fix up the check for the benefit of future
> readers.  A breakpoint is in the kernel if either end is in the
> kernel.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>

Indeed, in fact Oleg posted the same patch a long while ago but I eventually forgot to
track them.

See https://lkml.org/lkml/2013/11/24/44

Arm, arm64 and sh show the same issue and powerpc uses some obscure is_kernel_addr().

Eventually we should move this function to kernel/event/hw_breakpoint.c with a weak
tag to let archs override it like powerpc does. Although it seems not to care about the
breakpoint length so perhaps it's not correct.

In fact we should have some sort of generic in_kernel_va_range(start, length). This
reminds me something I'm sure it already exists :-)

> ---
>  arch/x86/kernel/hw_breakpoint.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index 6f345d302cf6..50a3fad5b89f 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -180,7 +180,11 @@ int arch_check_bp_in_kernelspace(struct perf_event *bp)
>  	va = info->address;
>  	len = bp->attr.bp_len;
>  
> -	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);
> +	/*
> +	 * We don't need to worry about va + len - 1 overflowing:
> +	 * we already require that va is aligned to a multiple of len.
> +	 */
> +	return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
>  }
>  
>  int arch_bp_generic_fields(int x86_len, int x86_type,
> -- 
> 2.4.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


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

FromAndy Lutomirski <luto@kernel.org>
Date2015-07-31 05:40 +0200
Subject[PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe
Message-ID<pS9kS-1Uh-9@gated-at.bofh.it>
In reply to#1196567
Code on the kprobe blacklist doesn't want unexpected int3
exceptions.  It probably doesn't want unexpected debug exceptions
either.  Be safe: disallow breakpoints in nokprobes code.

On non-CONFIG_KPROBES kernels, there is no kprobe blacklist.  In
that case, disallow kernel breakpoints entirely.

It will be particularly important to keep hw breakpoints out of the
entry and NMI code once we move debug exceptions off the IST stack.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/kernel/hw_breakpoint.c | 15 +++++++++++++++
 include/linux/kprobes.h         |  2 ++
 kernel/kprobes.c                |  2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 7114ba220fd4..78f3e90c5659 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -32,6 +32,7 @@
 #include <linux/irqflags.h>
 #include <linux/notifier.h>
 #include <linux/kallsyms.h>
+#include <linux/kprobes.h>
 #include <linux/percpu.h>
 #include <linux/kdebug.h>
 #include <linux/kernel.h>
@@ -243,6 +244,20 @@ 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 1ab54754a86d..8f6849084248 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,8 @@ 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);
+
 struct kprobe_insn_cache {
 	struct mutex mutex;
 	void *(*alloc)(void);	/* allocate insn page */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index c90e417bb963..d10ab6b9b5e0 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;
 }
 
-static bool within_kprobe_blacklist(unsigned long addr)
+bool within_kprobe_blacklist(unsigned long addr)
 {
 	struct kprobe_blacklist_entry *ent;
 
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1199566 — [tip:perf/core] perf/x86/hw_breakpoints: Disallow kernel breakpoints unless kprobe-safe

Fromtip-bot for Andy Lutomirski <tipbot@zytor.com>
Date2015-08-04 11:00 +0200
Subject[tip:perf/core] perf/x86/hw_breakpoints: Disallow kernel breakpoints unless kprobe-safe
Message-ID<pTGeM-5Af-41@gated-at.bofh.it>
In reply to#1196569
Commit-ID:  e5779e8e12299f77c2421a707855d8d124171d85
Gitweb:     http://git.kernel.org/tip/e5779e8e12299f77c2421a707855d8d124171d85
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Thu, 30 Jul 2015 20:32:40 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 4 Aug 2015 10:16:54 +0200

perf/x86/hw_breakpoints: Disallow kernel breakpoints unless kprobe-safe

Code on the kprobe blacklist doesn't want unexpected int3
exceptions. It probably doesn't want unexpected debug exceptions
either. Be safe: disallow breakpoints in nokprobes code.

On non-CONFIG_KPROBES kernels, there is no kprobe blacklist.  In
that case, disallow kernel breakpoints entirely.

It will be particularly important to keep hw breakpoints out of the
entry and NMI code once we move debug exceptions off the IST stack.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/e14b152af99640448d895e3c2a8c2d5ee19a1325.1438312874.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/hw_breakpoint.c | 15 +++++++++++++++
 include/linux/kprobes.h         |  2 ++
 kernel/kprobes.c                |  2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 7114ba2..78f3e90 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -32,6 +32,7 @@
 #include <linux/irqflags.h>
 #include <linux/notifier.h>
 #include <linux/kallsyms.h>
+#include <linux/kprobes.h>
 #include <linux/percpu.h>
 #include <linux/kdebug.h>
 #include <linux/kernel.h>
@@ -243,6 +244,20 @@ 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 1ab5475..8f68490 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,8 @@ 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);
+
 struct kprobe_insn_cache {
 	struct mutex mutex;
 	void *(*alloc)(void);	/* allocate insn page */
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index c90e417..d10ab6b 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;
 }
 
-static bool within_kprobe_blacklist(unsigned long addr)
+bool within_kprobe_blacklist(unsigned long addr)
 {
 	struct kprobe_blacklist_entry *ent;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


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

FromFrederic Weisbecker <fweisbec@gmail.com>
Date2015-08-04 18:00 +0200
SubjectRe: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe
Message-ID<pTMNd-6DQ-5@gated-at.bofh.it>
In reply to#1196569
On Thu, Jul 30, 2015 at 08:32:40PM -0700, Andy Lutomirski wrote:
> Code on the kprobe blacklist doesn't want unexpected int3
> exceptions.  It probably doesn't want unexpected debug exceptions
> either.  Be safe: disallow breakpoints in nokprobes code.
> 
> On non-CONFIG_KPROBES kernels, there is no kprobe blacklist.  In
> that case, disallow kernel breakpoints entirely.
> 
> It will be particularly important to keep hw breakpoints out of the
> entry and NMI code once we move debug exceptions off the IST stack.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  arch/x86/kernel/hw_breakpoint.c | 15 +++++++++++++++
>  include/linux/kprobes.h         |  2 ++
>  kernel/kprobes.c                |  2 +-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index 7114ba220fd4..78f3e90c5659 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -32,6 +32,7 @@
>  #include <linux/irqflags.h>
>  #include <linux/notifier.h>
>  #include <linux/kallsyms.h>
> +#include <linux/kprobes.h>
>  #include <linux/percpu.h>
>  #include <linux/kdebug.h>
>  #include <linux/kernel.h>
> @@ -243,6 +244,20 @@ 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
> +		}
> +

It should be done on generic code I think. In validate_hw_breakpoint()
under the arch_check_bp_in_kernelspace() check.

>  		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 1ab54754a86d..8f6849084248 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -267,6 +267,8 @@ 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);

The name was fine for a kprobe's private function. But if you make
it public, maybe standardize the prefix like kprobes_within_blacklist().

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1200316 — RE: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe

From平松雅巳 / HIRAMATU,MASAMI <masami.hiramatsu.pt@hitachi.com>
Date2015-08-05 01:30 +0200
SubjectRE: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe
Message-ID<pTTOF-kw-17@gated-at.bofh.it>
In reply to#1200117
Hi,

> From: Frederic Weisbecker [mailto:fweisbec@gmail.com]
> 
> On Thu, Jul 30, 2015 at 08:32:40PM -0700, Andy Lutomirski wrote:
> > Code on the kprobe blacklist doesn't want unexpected int3
> > exceptions.  It probably doesn't want unexpected debug exceptions
> > either.  Be safe: disallow breakpoints in nokprobes code.
> >
> > On non-CONFIG_KPROBES kernels, there is no kprobe blacklist.  In
> > that case, disallow kernel breakpoints entirely.
> >
> > It will be particularly important to keep hw breakpoints out of the
> > entry and NMI code once we move debug exceptions off the IST stack.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  arch/x86/kernel/hw_breakpoint.c | 15 +++++++++++++++
> >  include/linux/kprobes.h         |  2 ++
> >  kernel/kprobes.c                |  2 +-
> >  3 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> > index 7114ba220fd4..78f3e90c5659 100644
> > --- a/arch/x86/kernel/hw_breakpoint.c
> > +++ b/arch/x86/kernel/hw_breakpoint.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/irqflags.h>
> >  #include <linux/notifier.h>
> >  #include <linux/kallsyms.h>
> > +#include <linux/kprobes.h>
> >  #include <linux/percpu.h>
> >  #include <linux/kdebug.h>
> >  #include <linux/kernel.h>
> > @@ -243,6 +244,20 @@ 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
> > +		}
> > +
> 
> It should be done on generic code I think. In validate_hw_breakpoint()
> under the arch_check_bp_in_kernelspace() check.

Agreed, kprobes also does it in generic code.

> 
> >  		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 1ab54754a86d..8f6849084248 100644
> > --- a/include/linux/kprobes.h
> > +++ b/include/linux/kprobes.h
> > @@ -267,6 +267,8 @@ 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);
> 
> The name was fine for a kprobe's private function. But if you make
> it public, maybe standardize the prefix like kprobes_within_blacklist().

No, there is the "kprobe_blacklist", that function means 
"whether the address is within kprobe_blacklist or not?" like within_module_core.

Thank you,

> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web