Messages in this thread | | | Subject | Re: [PATCH 02/14] x86/hw_breakpoint: Prevent data breakpoints on direct GDT | From | Andrew Cooper <> | Date | Sat, 30 May 2020 13:45:43 +0100 |
| |
On 29/05/2020 22:27, Peter Zijlstra wrote: > From: Lai Jiangshan <laijs@linux.alibaba.com> > > A data breakpoint on the GDT is terrifying and should be avoided. > The GDT on CPU entry area is already protected. The direct GDT > should be also protected, although it is seldom used and only > used for short time.
While I agree with the sentiment...
> > Signed-off-by: Lai Jiangshan <laijs@linux.alibaba.com> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Link: https://lkml.kernel.org/r/20200526014221.2119-3-laijs@linux.alibaba.com > --- > arch/x86/kernel/hw_breakpoint.c | 30 ++++++++++++++++++++++-------- > 1 file changed, 22 insertions(+), 8 deletions(-) > > --- a/arch/x86/kernel/hw_breakpoint.c > +++ b/arch/x86/kernel/hw_breakpoint.c > @@ -32,6 +32,7 @@ > #include <asm/processor.h> > #include <asm/debugreg.h> > #include <asm/user.h> > +#include <asm/desc.h> > > /* Per cpu debug control register value */ > DEFINE_PER_CPU(unsigned long, cpu_dr7); > @@ -237,13 +238,26 @@ static inline bool within_area(unsigned > } > > /* > - * Checks whether the range from addr to end, inclusive, overlaps the CPU > - * entry area range. > + * Checks whether the range from addr to end, inclusive, overlaps the fixed > + * mapped CPU entry area range or other ranges used for CPU entry. > */ > -static inline bool within_cpu_entry_area(unsigned long addr, unsigned long end) > +static inline bool within_cpu_entry(unsigned long addr, unsigned long end) > { > - return within_area(addr, end, CPU_ENTRY_AREA_BASE, > - CPU_ENTRY_AREA_TOTAL_SIZE); > + int cpu; > + > + /* CPU entry erea is always used for CPU entry */ > + if (within_area(addr, end, CPU_ENTRY_AREA_BASE, > + CPU_ENTRY_AREA_TOTAL_SIZE)) > + return true; > + > + for_each_possible_cpu(cpu) { > + /* The original rw GDT is being used after load_direct_gdt() */ > + if (within_area(addr, end, (unsigned long)get_cpu_gdt_rw(cpu), > + GDT_SIZE))
... why the O(n) loop over the system?
It is only GDTs which might ever be active on this local CPU(/thread) which are a problem, because the breakpoint registers are similarly local.
Nothing is going to go wrong If I put a breakpoint on someone else's live GDT, because they wont interact in the "fun" ways we're trying to avoid.
~Andrew
| |