2015-07-31 03:32:47

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 0/3] perf: hw_breakpoint safety improvements

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


2015-07-31 03:32:49

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 1/3] x86/perf/hw_breakpoint: 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 <[email protected]>
---
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

2015-07-31 03:33:20

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 2/3] x86/perf/hw_breakpoint: Improve range breakpoint validation

Range breakpoints will do the wrong thing if the address isn't
aligned. While we're there, add comments about why it's safe for
instruction breakpoints.

Signed-off-by: Andy Lutomirski <[email protected]>
---
arch/x86/kernel/hw_breakpoint.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 78f3e90c5659..6f345d302cf6 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -291,8 +291,18 @@ static int arch_build_bp_info(struct perf_event *bp)
break;
#endif
default:
+ /* AMD range breakpoint */
if (!is_power_of_2(bp->attr.bp_len))
return -EINVAL;
+ if (bp->attr.bp_addr & (bp->attr.bp_len - 1))
+ return -EINVAL;
+ /*
+ * It's impossible to use a range breakpoint to fake out
+ * user vs kernel detection because bp_len - 1 can't
+ * have the high bit set. If we ever allow range instruction
+ * breakpoints, then we'll have to check for kprobe-blacklisted
+ * addresses anywhere in the range.
+ */
if (!cpu_has_bpext)
return -EOPNOTSUPP;
info->mask = bp->attr.bp_len - 1;
--
2.4.3

2015-07-31 03:32:52

by Andy Lutomirski

[permalink] [raw]
Subject: [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace 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 <[email protected]>
---
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

2015-07-31 08:10:47

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/3] perf: hw_breakpoint safety improvements


* Andy Lutomirski <[email protected]> wrote:

> 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(-)

Looks good to me at first glance. Cc:-ed Frederic who wrote and maintains these
bits. Frederic, what do you think?

Thanks,

Ingo

2015-07-31 08:22:00

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/3] perf: hw_breakpoint safety improvements

On Thu, Jul 30, 2015 at 08:32:39PM -0700, Andy Lutomirski wrote:
> Hi, Peter-
>
> Here are some baby steps toward eliminating nested NMIs. What do
> you think?

They work for me, Thanks!

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

Commit-ID: e5779e8e12299f77c2421a707855d8d124171d85
Gitweb: http://git.kernel.org/tip/e5779e8e12299f77c2421a707855d8d124171d85
Author: Andy Lutomirski <[email protected]>
AuthorDate: Thu, 30 Jul 2015 20:32:40 -0700
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/e14b152af99640448d895e3c2a8c2d5ee19a1325.1438312874.git.luto@kernel.org
Signed-off-by: Ingo Molnar <[email protected]>
---
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;

Subject: [tip:perf/core] perf/x86/hw_breakpoints: Improve range breakpoint validation

Commit-ID: ab513927ab449af00cc70b0269e15ee80dd537f9
Gitweb: http://git.kernel.org/tip/ab513927ab449af00cc70b0269e15ee80dd537f9
Author: Andy Lutomirski <[email protected]>
AuthorDate: Thu, 30 Jul 2015 20:32:41 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 4 Aug 2015 10:16:54 +0200

perf/x86/hw_breakpoints: Improve range breakpoint validation

Range breakpoints will do the wrong thing if the address isn't
aligned. While we're there, add comments about why it's safe for
instruction breakpoints.

Signed-off-by: Andy Lutomirski <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/ae25d14d61f2f43b78e0a247e469f3072df7e201.1438312874.git.luto@kernel.org
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/hw_breakpoint.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 78f3e90..6f345d3 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -291,8 +291,18 @@ static int arch_build_bp_info(struct perf_event *bp)
break;
#endif
default:
+ /* AMD range breakpoint */
if (!is_power_of_2(bp->attr.bp_len))
return -EINVAL;
+ if (bp->attr.bp_addr & (bp->attr.bp_len - 1))
+ return -EINVAL;
+ /*
+ * It's impossible to use a range breakpoint to fake out
+ * user vs kernel detection because bp_len - 1 can't
+ * have the high bit set. If we ever allow range instruction
+ * breakpoints, then we'll have to check for kprobe-blacklisted
+ * addresses anywhere in the range.
+ */
if (!cpu_has_bpext)
return -EOPNOTSUPP;
info->mask = bp->attr.bp_len - 1;

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

Commit-ID: 27747f8bc355a2808ca9e490ab6866acd85b4c16
Gitweb: http://git.kernel.org/tip/27747f8bc355a2808ca9e490ab6866acd85b4c16
Author: Andy Lutomirski <[email protected]>
AuthorDate: Thu, 30 Jul 2015 20:32:42 -0700
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/136be387950e78f18cea60e9d1bef74465d0ee8f.1438312874.git.luto@kernel.org
Signed-off-by: Ingo Molnar <[email protected]>
---
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,

2015-08-04 15:52:04

by Frederic Weisbecker

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

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 <[email protected]>
> ---
> 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.

2015-08-04 16:13:59

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH 3/3] x86/perf/hw_breakpoint: Fix check for kernelspace breakpoints

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 <[email protected]>

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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

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

Hi,

> From: Frederic Weisbecker [mailto:[email protected]]
>
> 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 <[email protected]>
> > ---
> > 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,

>