Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1216774 > unrolled thread
| Started by | Frederic Weisbecker <fweisbec@gmail.com> |
|---|---|
| First post | 2015-09-01 13:00 +0200 |
| Last post | 2015-09-01 13:50 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe Frederic Weisbecker <fweisbec@gmail.com> - 2015-09-01 13:00 +0200
Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe Peter Zijlstra <peterz@infradead.org> - 2015-09-01 13:40 +0200
Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe Peter Zijlstra <peterz@infradead.org> - 2015-09-01 13:50 +0200
| From | Frederic Weisbecker <fweisbec@gmail.com> |
|---|---|
| Date | 2015-09-01 13:00 +0200 |
| Subject | Re: [PATCH 1/3] x86/perf/hw_breakpoint: Disallow kernel breakpoints unless kprobe-safe |
| Message-ID | <q3Rsd-4kv-7@gated-at.bofh.it> |
2015-08-05 1:29 GMT+02:00 平松雅巳 / HIRAMATU，MASAMI
<masami.hiramatsu.pt@hitachi.com>:
> 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.
Well, the patchset got applied anyway and the reviews ignored...
--
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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-09-01 13:40 +0200 |
| Message-ID | <q3S4W-5ix-23@gated-at.bofh.it> |
| In reply to | #1216774 |
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;
--
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]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-09-01 13:50 +0200 |
| Message-ID | <q3SeB-5tT-3@gated-at.bofh.it> |
| In reply to | #1216788 |
On Tue, Sep 01, 2015 at 01:36:08PM +0200, Peter Zijlstra wrote:
> +static inline bool kprobe_blacklisted(unsigned long addr)
> +{
> + return false;
Ah, I think that wants to be true instead, to match the below comment.
> +}
> +
> 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;
--
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