From: Andy Lutomirski <[email protected]>
A data breakpoint near the top of an IST stack will cause unresoverable
recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
Prevent either of these from happening.
Co-developed-by: Peter Zijlstra <[email protected]>
Signed-off-by: Andy Lutomirski <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/hw_breakpoint.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -227,10 +227,35 @@ int arch_check_bp_in_kernelspace(struct
return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
}
+/*
+ * Checks whether the range from addr to end, inclusive, overlaps the CPU
+ * entry area range.
+ */
+static inline bool within_cpu_entry_area(unsigned long addr, unsigned long end)
+{
+ return end >= CPU_ENTRY_AREA_PER_CPU &&
+ addr < (CPU_ENTRY_AREA_PER_CPU + CPU_ENTRY_AREA_TOTAL_SIZE);
+}
+
static int arch_build_bp_info(struct perf_event *bp,
const struct perf_event_attr *attr,
struct arch_hw_breakpoint *hw)
{
+ unsigned long bp_end;
+
+ bp_end = attr->bp_addr + attr->bp_len - 1;
+ if (bp_end < attr->bp_addr)
+ return -EINVAL;
+
+ /*
+ * Prevent any breakpoint of any type that overlaps the
+ * cpu_entry_area. This protects the IST stacks and also
+ * reduces the chance that we ever find out what happens if
+ * there's a data breakpoint on the GDT, IDT, or TSS.
+ */
+ if (within_cpu_entry_area(attr->bp_addr, bp_end))
+ return -EINVAL;
+
hw->address = attr->bp_addr;
hw->mask = 0;
On Tue, May 05, 2020 at 03:16:04PM +0200, Thomas Gleixner wrote:
> From: Andy Lutomirski <[email protected]>
>
> A data breakpoint near the top of an IST stack will cause unresoverable
unrecoverable
> recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
"terrifying" huh? Colorful. :)
> Prevent either of these from happening.
>
> Co-developed-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/hw_breakpoint.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
Reviewed-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 5/5/20 3:16 PM, Thomas Gleixner wrote:
> From: Andy Lutomirski <[email protected]>
>
> A data breakpoint near the top of an IST stack will cause unresoverable
typo: unresoverable -> unrecoverable
> recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
> Prevent either of these from happening.
>
> Co-developed-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/hw_breakpoint.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
Reviewed-by: Alexandre Chartre <[email protected]>
alex.
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -227,10 +227,35 @@ int arch_check_bp_in_kernelspace(struct
> return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
> }
>
> +/*
> + * Checks whether the range from addr to end, inclusive, overlaps the CPU
> + * entry area range.
> + */
> +static inline bool within_cpu_entry_area(unsigned long addr, unsigned long end)
> +{
> + return end >= CPU_ENTRY_AREA_PER_CPU &&
> + addr < (CPU_ENTRY_AREA_PER_CPU + CPU_ENTRY_AREA_TOTAL_SIZE);
> +}
> +
> static int arch_build_bp_info(struct perf_event *bp,
> const struct perf_event_attr *attr,
> struct arch_hw_breakpoint *hw)
> {
> + unsigned long bp_end;
> +
> + bp_end = attr->bp_addr + attr->bp_len - 1;
> + if (bp_end < attr->bp_addr)
> + return -EINVAL;
> +
> + /*
> + * Prevent any breakpoint of any type that overlaps the
> + * cpu_entry_area. This protects the IST stacks and also
> + * reduces the chance that we ever find out what happens if
> + * there's a data breakpoint on the GDT, IDT, or TSS.
> + */
> + if (within_cpu_entry_area(attr->bp_addr, bp_end))
> + return -EINVAL;
> +
> hw->address = attr->bp_addr;
> hw->mask = 0;
>
>
On Tue, May 5, 2020 at 10:15 PM Thomas Gleixner <[email protected]> wrote:
>
> From: Andy Lutomirski <[email protected]>
>
> A data breakpoint near the top of an IST stack will cause unresoverable
> recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
> Prevent either of these from happening.
>
> Co-developed-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/hw_breakpoint.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -227,10 +227,35 @@ int arch_check_bp_in_kernelspace(struct
> return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
> }
>
> +/*
> + * Checks whether the range from addr to end, inclusive, overlaps the CPU
> + * entry area range.
> + */
> +static inline bool within_cpu_entry_area(unsigned long addr, unsigned long end)
> +{
> + return end >= CPU_ENTRY_AREA_PER_CPU &&
> + addr < (CPU_ENTRY_AREA_PER_CPU + CPU_ENTRY_AREA_TOTAL_SIZE);
> +}
Hello
These two lines:
s/CPU_ENTRY_AREA_PER_CPU/CPU_ENTRY_AREA_BASE/g
or
s/CPU_ENTRY_AREA_PER_CPU/CPU_ENTRY_AREA_RO_IDT/g
or otherwise the RO_IDT is not being protected.
sees:
#define CPU_ENTRY_AREA_PER_CPU (CPU_ENTRY_AREA_RO_IDT + PAGE_SIZE)
#define CPU_ENTRY_AREA_TOTAL_SIZE (CPU_ENTRY_AREA_ARRAY_SIZE + PAGE_SIZE)
^ sizeof PER_CPU ^ RO_IDT
Reviewed-by: Lai Jiangshan <[email protected]>
> +
> static int arch_build_bp_info(struct perf_event *bp,
> const struct perf_event_attr *attr,
> struct arch_hw_breakpoint *hw)
> {
> + unsigned long bp_end;
> +
> + bp_end = attr->bp_addr + attr->bp_len - 1;
> + if (bp_end < attr->bp_addr)
> + return -EINVAL;
> +
> + /*
> + * Prevent any breakpoint of any type that overlaps the
> + * cpu_entry_area. This protects the IST stacks and also
> + * reduces the chance that we ever find out what happens if
> + * there's a data breakpoint on the GDT, IDT, or TSS.
> + */
> + if (within_cpu_entry_area(attr->bp_addr, bp_end))
> + return -EINVAL;
> +
> hw->address = attr->bp_addr;
> hw->mask = 0;
>
>
On Tue, May 5, 2020 at 10:15 PM Thomas Gleixner <[email protected]> wrote:
>
> From: Andy Lutomirski <[email protected]>
>
> A data breakpoint near the top of an IST stack will cause unresoverable
> recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
> Prevent either of these from happening.
>
What happen when a data breakpoint on the direct GDT (load_direct_gdt())
and the debug IDT (load_debug_idt()) which are not considered in this patch?
Thanks
Lai
On Sat, May 9, 2020 at 2:23 AM Lai Jiangshan
<[email protected]> wrote:
>
> On Tue, May 5, 2020 at 10:15 PM Thomas Gleixner <[email protected]> wrote:
> >
> > From: Andy Lutomirski <[email protected]>
> >
> > A data breakpoint near the top of an IST stack will cause unresoverable
> > recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
> > Prevent either of these from happening.
> >
>
> What happen when a data breakpoint on the direct GDT (load_direct_gdt())
> and the debug IDT (load_debug_idt()) which are not considered in this patch?
>
I have no idea, and learning the answer may involve talking to the
respective CPU vendors' microcode engineers. We should probably block
those, too.
The following commit has been merged into the x86/entry branch of tip:
Commit-ID: 3ea11ac991d594728e5df42f7eb1145072b9c2bc
Gitweb: https://git.kernel.org/tip/3ea11ac991d594728e5df42f7eb1145072b9c2bc
Author: Andy Lutomirski <[email protected]>
AuthorDate: Mon, 24 Feb 2020 13:24:58 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Fri, 15 May 2020 20:03:03 +02:00
x86/hw_breakpoint: Prevent data breakpoints on cpu_entry_area
A data breakpoint near the top of an IST stack will cause unrecoverable
recursion. A data breakpoint on the GDT, IDT, or TSS is terrifying.
Prevent either of these from happening.
Co-developed-by: Peter Zijlstra <[email protected]>
Signed-off-by: Andy Lutomirski <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Borislav Petkov <[email protected]>
Reviewed-by: Lai Jiangshan <[email protected]>
Reviewed-by: Alexandre Chartre <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/kernel/hw_breakpoint.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 4d8d53e..d42fc0e 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -227,10 +227,35 @@ int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw)
return (va >= TASK_SIZE_MAX) || ((va + len - 1) >= TASK_SIZE_MAX);
}
+/*
+ * Checks whether the range from addr to end, inclusive, overlaps the CPU
+ * entry area range.
+ */
+static inline bool within_cpu_entry_area(unsigned long addr, unsigned long end)
+{
+ return end >= CPU_ENTRY_AREA_BASE &&
+ addr < (CPU_ENTRY_AREA_BASE + CPU_ENTRY_AREA_TOTAL_SIZE);
+}
+
static int arch_build_bp_info(struct perf_event *bp,
const struct perf_event_attr *attr,
struct arch_hw_breakpoint *hw)
{
+ unsigned long bp_end;
+
+ bp_end = attr->bp_addr + attr->bp_len - 1;
+ if (bp_end < attr->bp_addr)
+ return -EINVAL;
+
+ /*
+ * Prevent any breakpoint of any type that overlaps the
+ * cpu_entry_area. This protects the IST stacks and also
+ * reduces the chance that we ever find out what happens if
+ * there's a data breakpoint on the GDT, IDT, or TSS.
+ */
+ if (within_cpu_entry_area(attr->bp_addr, bp_end))
+ return -EINVAL;
+
hw->address = attr->bp_addr;
hw->mask = 0;