2020-05-05 14:19:34

by Thomas Gleixner

[permalink] [raw]
Subject: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

From: Peter Zijlstra <[email protected]>

DR6/7 should be handled before nmi_enter() is invoked and restore after
nmi_exit() to minimize the exposure.

Split it out into helper inlines and bring it into the correct order.

Signed-off-by: Peter Zijlstra <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/hw_breakpoint.c | 6 ---
arch/x86/kernel/traps.c | 62 +++++++++++++++++++++++++++-------------
2 files changed, 44 insertions(+), 24 deletions(-)

--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -464,7 +464,7 @@ static int hw_breakpoint_handler(struct
{
int i, cpu, rc = NOTIFY_STOP;
struct perf_event *bp;
- unsigned long dr7, dr6;
+ unsigned long dr6;
unsigned long *dr6_p;

/* The DR6 value is pointed by args->err */
@@ -479,9 +479,6 @@ static int hw_breakpoint_handler(struct
if ((dr6 & DR_TRAP_BITS) == 0)
return NOTIFY_DONE;

- get_debugreg(dr7, 7);
- /* Disable breakpoints during exception handling */
- set_debugreg(0UL, 7);
/*
* Assert that local interrupts are disabled
* Reset the DRn bits in the virtualized register value.
@@ -538,7 +535,6 @@ static int hw_breakpoint_handler(struct
(dr6 & (~DR_TRAP_BITS)))
rc = NOTIFY_DONE;

- set_debugreg(dr7, 7);
put_cpu();

return rc;
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -691,6 +691,44 @@ static bool is_sysenter_singlestep(struc
#endif
}

+static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
+{
+ /*
+ * Disable breakpoints during exception handling; recursive exceptions
+ * are exceedingly 'fun'.
+ *
+ * Since this function is NOKPROBE, and that also applies to
+ * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
+ * HW_BREAKPOINT_W on our stack)
+ *
+ * Entry text is excluded for HW_BP_X and cpu_entry_area, which
+ * includes the entry stack is excluded for everything.
+ */
+ get_debugreg(*dr7, 6);
+ set_debugreg(0, 7);
+
+ /*
+ * The Intel SDM says:
+ *
+ * Certain debug exceptions may clear bits 0-3. The remaining
+ * contents of the DR6 register are never cleared by the
+ * processor. To avoid confusion in identifying debug
+ * exceptions, debug handlers should clear the register before
+ * returning to the interrupted task.
+ *
+ * Keep it simple: clear DR6 immediately.
+ */
+ get_debugreg(*dr6, 6);
+ set_debugreg(0, 6);
+ /* Filter out all the reserved bits which are preset to 1 */
+ *dr6 &= ~DR6_RESERVED;
+}
+
+static __always_inline void debug_exit(unsigned long dr7)
+{
+ set_debugreg(dr7, 7);
+}
+
/*
* Our handling of the processor debug registers is non-trivial.
* We do not clear them on entry and exit from the kernel. Therefore
@@ -718,28 +756,13 @@ static bool is_sysenter_singlestep(struc
dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
{
struct task_struct *tsk = current;
+ unsigned long dr6, dr7;
int user_icebp = 0;
- unsigned long dr6;
int si_code;

- nmi_enter();
-
- get_debugreg(dr6, 6);
- /*
- * The Intel SDM says:
- *
- * Certain debug exceptions may clear bits 0-3. The remaining
- * contents of the DR6 register are never cleared by the
- * processor. To avoid confusion in identifying debug
- * exceptions, debug handlers should clear the register before
- * returning to the interrupted task.
- *
- * Keep it simple: clear DR6 immediately.
- */
- set_debugreg(0, 6);
+ debug_enter(&dr6, &dr7);

- /* Filter out all the reserved bits which are preset to 1 */
- dr6 &= ~DR6_RESERVED;
+ nmi_enter();

/*
* The SDM says "The processor clears the BTF flag when it
@@ -777,7 +800,7 @@ dotraplinkage void do_debug(struct pt_re
#endif

if (notify_die(DIE_DEBUG, "debug", regs, (long)&dr6, error_code,
- SIGTRAP) == NOTIFY_STOP)
+ SIGTRAP) == NOTIFY_STOP)
goto exit;

/*
@@ -816,6 +839,7 @@ dotraplinkage void do_debug(struct pt_re

exit:
nmi_exit();
+ debug_exit(dr7);
}
NOKPROBE_SYMBOL(do_debug);



2020-05-07 17:25:28

by Alexandre Chartre

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling


On 5/5/20 3:49 PM, Thomas Gleixner wrote:
> From: Peter Zijlstra <[email protected]>
>
> DR6/7 should be handled before nmi_enter() is invoked and restore after
> nmi_exit() to minimize the exposure.
>
> Split it out into helper inlines and bring it into the correct order.
>
> Signed-off-by: Peter Zijlstra <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/kernel/hw_breakpoint.c | 6 ---
> arch/x86/kernel/traps.c | 62 +++++++++++++++++++++++++++-------------
> 2 files changed, 44 insertions(+), 24 deletions(-)
>
...
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -691,6 +691,44 @@ static bool is_sysenter_singlestep(struc
> #endif
> }
>
> +static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
> +{
> + /*
> + * Disable breakpoints during exception handling; recursive exceptions
> + * are exceedingly 'fun'.
> + *
> + * Since this function is NOKPROBE, and that also applies to
> + * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
> + * HW_BREAKPOINT_W on our stack)
> + *
> + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> + * includes the entry stack is excluded for everything.
> + */
> + get_debugreg(*dr7, 6);

Do you mean get_debugreg(*dr7, 7); ?

alex.

2020-05-08 09:04:12

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

On Thu, May 07, 2020 at 07:18:45PM +0200, Alexandre Chartre wrote:
>
> On 5/5/20 3:49 PM, Thomas Gleixner wrote:
> > From: Peter Zijlstra <[email protected]>
> >
> > DR6/7 should be handled before nmi_enter() is invoked and restore after
> > nmi_exit() to minimize the exposure.
> >
> > Split it out into helper inlines and bring it into the correct order.
> >
> > Signed-off-by: Peter Zijlstra <[email protected]>
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > ---
> > arch/x86/kernel/hw_breakpoint.c | 6 ---
> > arch/x86/kernel/traps.c | 62 +++++++++++++++++++++++++++-------------
> > 2 files changed, 44 insertions(+), 24 deletions(-)
> >
> ...
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -691,6 +691,44 @@ static bool is_sysenter_singlestep(struc
> > #endif
> > }
> > +static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
> > +{
> > + /*
> > + * Disable breakpoints during exception handling; recursive exceptions
> > + * are exceedingly 'fun'.
> > + *
> > + * Since this function is NOKPROBE, and that also applies to
> > + * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
> > + * HW_BREAKPOINT_W on our stack)
> > + *
> > + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> > + * includes the entry stack is excluded for everything.
> > + */
> > + get_debugreg(*dr7, 6);
>
> Do you mean get_debugreg(*dr7, 7); ?

Shees, I have to go buy a new stack of brown paper bags at this rate,
don't I :/

2020-05-08 12:02:45

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

Peter Zijlstra <[email protected]> writes:
>> > +static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
>> > +{
>> > + /*
>> > + * Disable breakpoints during exception handling; recursive exceptions
>> > + * are exceedingly 'fun'.
>> > + *
>> > + * Since this function is NOKPROBE, and that also applies to
>> > + * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
>> > + * HW_BREAKPOINT_W on our stack)
>> > + *
>> > + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
>> > + * includes the entry stack is excluded for everything.
>> > + */
>> > + get_debugreg(*dr7, 6);
>>
>> Do you mean get_debugreg(*dr7, 7); ?
>
> Shees, I have to go buy a new stack of brown paper bags at this rate,
> don't I :/

Not only you, but it's also amazing that the selftests didn't catch
that.

2020-05-08 13:16:56

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

On Fri, May 08, 2020 at 01:58:30PM +0200, Thomas Gleixner wrote:
> Peter Zijlstra <[email protected]> writes:
> >> > +static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
> >> > +{
> >> > + /*
> >> > + * Disable breakpoints during exception handling; recursive exceptions
> >> > + * are exceedingly 'fun'.
> >> > + *
> >> > + * Since this function is NOKPROBE, and that also applies to
> >> > + * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
> >> > + * HW_BREAKPOINT_W on our stack)
> >> > + *
> >> > + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> >> > + * includes the entry stack is excluded for everything.
> >> > + */
> >> > + get_debugreg(*dr7, 6);
> >>
> >> Do you mean get_debugreg(*dr7, 7); ?
> >
> > Shees, I have to go buy a new stack of brown paper bags at this rate,
> > don't I :/
>
> Not only you, but it's also amazing that the selftests didn't catch
> that.

I don't think the selftests try and set hardware breakpoints in the
kernel.

2020-05-14 02:27:45

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

----- On May 5, 2020, at 9:49 AM, Thomas Gleixner [email protected] wrote:

> From: Peter Zijlstra <[email protected]>
>
> DR6/7 should be handled before nmi_enter() is invoked and restore after
> nmi_exit() to minimize the exposure.
>
> Split it out into helper inlines and bring it into the correct order.
>
[...]
>
> +static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
> +{
> + /*
> + * Disable breakpoints during exception handling; recursive exceptions
> + * are exceedingly 'fun'.
> + *
> + * Since this function is NOKPROBE, and that also applies to
> + * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
> + * HW_BREAKPOINT_W on our stack)
> + *
> + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> + * includes the entry stack is excluded for everything.
> + */
> + get_debugreg(*dr7, 6);
> + set_debugreg(0, 7);
> +
> + /*
> + * The Intel SDM says:
> + *
> + * Certain debug exceptions may clear bits 0-3. The remaining
> + * contents of the DR6 register are never cleared by the
> + * processor. To avoid confusion in identifying debug
> + * exceptions, debug handlers should clear the register before
> + * returning to the interrupted task.
> + *
> + * Keep it simple: clear DR6 immediately.
> + */
> + get_debugreg(*dr6, 6);
> + set_debugreg(0, 6);
> + /* Filter out all the reserved bits which are preset to 1 */
> + *dr6 &= ~DR6_RESERVED;
> +}
> +
> +static __always_inline void debug_exit(unsigned long dr7)
> +{
> + set_debugreg(dr7, 7);
> +}

Out of curiosity, what prevents the compiler from moving instructions
outside of the code regions surrounded by entry/exit ? This is an always
inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
which in turn uses an asm() (not volatile), without any memory clobber.

Also, considering that "inline" is not sufficient to ensure the compiler
does not emit a traceable function, I suspect you'll also want to mark
"native_get_debugreg" and "native_set_debugreg" always inline as well.

Thanks,

Mathieu

> +
> /*
> * Our handling of the processor debug registers is non-trivial.
> * We do not clear them on entry and exit from the kernel. Therefore
> @@ -718,28 +756,13 @@ static bool is_sysenter_singlestep(struc
> dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
> {
> struct task_struct *tsk = current;
> + unsigned long dr6, dr7;
> int user_icebp = 0;
> - unsigned long dr6;
> int si_code;
>
> - nmi_enter();
> -
> - get_debugreg(dr6, 6);
> - /*
> - * The Intel SDM says:
> - *
> - * Certain debug exceptions may clear bits 0-3. The remaining
> - * contents of the DR6 register are never cleared by the
> - * processor. To avoid confusion in identifying debug
> - * exceptions, debug handlers should clear the register before
> - * returning to the interrupted task.
> - *
> - * Keep it simple: clear DR6 immediately.
> - */
> - set_debugreg(0, 6);
> + debug_enter(&dr6, &dr7);
>
> - /* Filter out all the reserved bits which are preset to 1 */
> - dr6 &= ~DR6_RESERVED;
> + nmi_enter();
>
> /*
> * The SDM says "The processor clears the BTF flag when it
> @@ -777,7 +800,7 @@ dotraplinkage void do_debug(struct pt_re
> #endif
>
> if (notify_die(DIE_DEBUG, "debug", regs, (long)&dr6, error_code,
> - SIGTRAP) == NOTIFY_STOP)
> + SIGTRAP) == NOTIFY_STOP)
> goto exit;
>
> /*
> @@ -816,6 +839,7 @@ dotraplinkage void do_debug(struct pt_re
>
> exit:
> nmi_exit();
> + debug_exit(dr7);
> }
> NOKPROBE_SYMBOL(do_debug);

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2020-05-14 17:31:01

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

Mathieu Desnoyers <[email protected]> writes:
> ----- On May 5, 2020, at 9:49 AM, Thomas Gleixner [email protected] wrote:
>> +
>> +static __always_inline void debug_exit(unsigned long dr7)
>> +{
>> + set_debugreg(dr7, 7);
>> +}
>
> Out of curiosity, what prevents the compiler from moving instructions
> outside of the code regions surrounded by entry/exit ? This is an always
> inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
> which in turn uses an asm() (not volatile), without any memory
> clobber.
>
> Also, considering that "inline" is not sufficient to ensure the compiler
> does not emit a traceable function, I suspect you'll also want to mark
> "native_get_debugreg" and "native_set_debugreg" always inline as well.

It can move it into a function and call that. Fine. If that function
stays in the noinstr section then it should not emit a trace stub and if
it moves it out of the section or reuses another instance in text then
objtool will complain.

Checking for trace stubs and other instrumentation nonsense is on the
objtool wishlist anyway.

But yes, marking these __always_inline prevents that. Those escaped my
chase. But I would have found them once I go and fix that paravirt muck.

Thanks,

tglx


2020-05-14 17:48:40

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

----- On May 14, 2020, at 1:28 PM, Thomas Gleixner [email protected] wrote:

> Mathieu Desnoyers <[email protected]> writes:
>> ----- On May 5, 2020, at 9:49 AM, Thomas Gleixner [email protected] wrote:
>>> +
>>> +static __always_inline void debug_exit(unsigned long dr7)
>>> +{
>>> + set_debugreg(dr7, 7);
>>> +}
>>

* Question 1

>> Out of curiosity, what prevents the compiler from moving instructions
>> outside of the code regions surrounded by entry/exit ? This is an always
>> inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
>> which in turn uses an asm() (not volatile), without any memory
>> clobber.
>>

?

* Question 2

>> Also, considering that "inline" is not sufficient to ensure the compiler
>> does not emit a traceable function, I suspect you'll also want to mark
>> "native_get_debugreg" and "native_set_debugreg" always inline as well.
>
> It can move it into a function and call that. Fine. If that function
> stays in the noinstr section then it should not emit a trace stub and if
> it moves it out of the section or reuses another instance in text then
> objtool will complain.
>
> Checking for trace stubs and other instrumentation nonsense is on the
> objtool wishlist anyway.
>
> But yes, marking these __always_inline prevents that. Those escaped my
> chase. But I would have found them once I go and fix that paravirt muck.

This answers only my second question (see "Question 1" above).

Thanks,

Mathieu


--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2020-05-14 18:09:57

by Steven Rostedt

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

On Wed, 13 May 2020 22:24:56 -0400 (EDT)
Mathieu Desnoyers <[email protected]> wrote:

> Also, considering that "inline" is not sufficient to ensure the compiler
> does not emit a traceable function, I suspect you'll also want to mark
> "native_get_debugreg" and "native_set_debugreg" always inline as well.

I was thinking that the noinstr sections was more about not doing tracing
and kprobes. As "inline" has been defined as "notrace" for some time, where
any function marked as "inline" will not be available to ftrace even if the
compiler decides not to honor the inline.

in linux/compiler_types.h:

#define inline inline __gnu_inline __inline_maybe_unused notrace

-- Steve

2020-05-15 05:39:54

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

On Tue, May 5, 2020 at 7:16 AM Thomas Gleixner <[email protected]> wrote:
>
> From: Peter Zijlstra <[email protected]>
>
> DR6/7 should be handled before nmi_enter() is invoked and restore after
> nmi_exit() to minimize the exposure.
>
> Split it out into helper inlines and bring it into the correct order.

> + *
> + * Entry text is excluded for HW_BP_X and cpu_entry_area, which
> + * includes the entry stack is excluded for everything.
> + */
> + get_debugreg(*dr7, 6);
> + set_debugreg(0, 7);

Fortunately, PeterZ is hiding in a brown paper bag, so I don't have to
comment :)

Other than that:

Acked-by: Andy Lutomirski <[email protected]>

2020-05-15 14:34:55

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch V4 part 4 15/24] x86/db: Split out dr6/7 handling

Mathieu Desnoyers <[email protected]> writes:

> ----- On May 14, 2020, at 1:28 PM, Thomas Gleixner [email protected] wrote:
>
>> Mathieu Desnoyers <[email protected]> writes:
>>> ----- On May 5, 2020, at 9:49 AM, Thomas Gleixner [email protected] wrote:
>>>> +
>>>> +static __always_inline void debug_exit(unsigned long dr7)
>>>> +{
>>>> + set_debugreg(dr7, 7);
>>>> +}
>>>
>
> * Question 1
>
>>> Out of curiosity, what prevents the compiler from moving instructions
>>> outside of the code regions surrounded by entry/exit ? This is an always
>>> inline, which invokes set_debugreg which is inline for CONFIG_PARAVIRT_XXL=n,
>>> which in turn uses an asm() (not volatile), without any memory
>>> clobber.

I misread 'surrounded by entry/exit'.

Reading it again I assume you mean nmi_enter/exit(). And yes, there is a
compiler barrier missing.

Thanks,

tglx

8<----------------
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index e11ad0791dc3..ae1e61345225 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -718,6 +718,13 @@ static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
get_debugreg(*dr7, 7);
set_debugreg(0, 7);

+ /*
+ * Ensure the compiler doesn't lower the above statements into
+ * the critical section; disabling breakpoints late would not
+ * be good.
+ */
+ barrier();
+
/*
* The Intel SDM says:
*
@@ -737,6 +744,12 @@ static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)

static __always_inline void debug_exit(unsigned long dr7)
{
+ /*
+ * Ensure the compiler doesn't raise this statement into
+ * the critical section; enabling breakpoints early would
+ * not be good.
+ */
+ barrier();
set_debugreg(dr7, 7);
}

2020-05-19 20:05:59

by tip-bot2 for Kees Cook

[permalink] [raw]
Subject: [tip: x86/entry] x86/db: Split out dr6/7 handling

The following commit has been merged into the x86/entry branch of tip:

Commit-ID: 9a3d7c76d28e55173be5dd7cadbe8760fb814afa
Gitweb: https://git.kernel.org/tip/9a3d7c76d28e55173be5dd7cadbe8760fb814afa
Author: Peter Zijlstra <[email protected]>
AuthorDate: Mon, 06 Apr 2020 21:02:56 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 19 May 2020 16:04:10 +02:00

x86/db: Split out dr6/7 handling

DR6/7 should be handled before nmi_enter() is invoked and restore after
nmi_exit() to minimize the exposure.

Split it out into helper inlines and bring it into the correct order.

Signed-off-by: Peter Zijlstra <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Alexandre Chartre <[email protected]>
Acked-by: Andy Lutomirski <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]


---
arch/x86/kernel/hw_breakpoint.c | 6 +---
arch/x86/kernel/traps.c | 75 +++++++++++++++++++++++---------
2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index d42fc0e..9ddf441 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -464,7 +464,7 @@ static int hw_breakpoint_handler(struct die_args *args)
{
int i, cpu, rc = NOTIFY_STOP;
struct perf_event *bp;
- unsigned long dr7, dr6;
+ unsigned long dr6;
unsigned long *dr6_p;

/* The DR6 value is pointed by args->err */
@@ -479,9 +479,6 @@ static int hw_breakpoint_handler(struct die_args *args)
if ((dr6 & DR_TRAP_BITS) == 0)
return NOTIFY_DONE;

- get_debugreg(dr7, 7);
- /* Disable breakpoints during exception handling */
- set_debugreg(0UL, 7);
/*
* Assert that local interrupts are disabled
* Reset the DRn bits in the virtualized register value.
@@ -538,7 +535,6 @@ static int hw_breakpoint_handler(struct die_args *args)
(dr6 & (~DR_TRAP_BITS)))
rc = NOTIFY_DONE;

- set_debugreg(dr7, 7);
put_cpu();

return rc;
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 21c8cfc..de5120e 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -700,6 +700,57 @@ static bool is_sysenter_singlestep(struct pt_regs *regs)
#endif
}

+static __always_inline void debug_enter(unsigned long *dr6, unsigned long *dr7)
+{
+ /*
+ * Disable breakpoints during exception handling; recursive exceptions
+ * are exceedingly 'fun'.
+ *
+ * Since this function is NOKPROBE, and that also applies to
+ * HW_BREAKPOINT_X, we can't hit a breakpoint before this (XXX except a
+ * HW_BREAKPOINT_W on our stack)
+ *
+ * Entry text is excluded for HW_BP_X and cpu_entry_area, which
+ * includes the entry stack is excluded for everything.
+ */
+ get_debugreg(*dr7, 7);
+ set_debugreg(0, 7);
+
+ /*
+ * Ensure the compiler doesn't lower the above statements into
+ * the critical section; disabling breakpoints late would not
+ * be good.
+ */
+ barrier();
+
+ /*
+ * The Intel SDM says:
+ *
+ * Certain debug exceptions may clear bits 0-3. The remaining
+ * contents of the DR6 register are never cleared by the
+ * processor. To avoid confusion in identifying debug
+ * exceptions, debug handlers should clear the register before
+ * returning to the interrupted task.
+ *
+ * Keep it simple: clear DR6 immediately.
+ */
+ get_debugreg(*dr6, 6);
+ set_debugreg(0, 6);
+ /* Filter out all the reserved bits which are preset to 1 */
+ *dr6 &= ~DR6_RESERVED;
+}
+
+static __always_inline void debug_exit(unsigned long dr7)
+{
+ /*
+ * Ensure the compiler doesn't raise this statement into
+ * the critical section; enabling breakpoints early would
+ * not be good.
+ */
+ barrier();
+ set_debugreg(dr7, 7);
+}
+
/*
* Our handling of the processor debug registers is non-trivial.
* We do not clear them on entry and exit from the kernel. Therefore
@@ -727,28 +778,13 @@ static bool is_sysenter_singlestep(struct pt_regs *regs)
dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
{
struct task_struct *tsk = current;
+ unsigned long dr6, dr7;
int user_icebp = 0;
- unsigned long dr6;
int si_code;

- nmi_enter();
-
- get_debugreg(dr6, 6);
- /*
- * The Intel SDM says:
- *
- * Certain debug exceptions may clear bits 0-3. The remaining
- * contents of the DR6 register are never cleared by the
- * processor. To avoid confusion in identifying debug
- * exceptions, debug handlers should clear the register before
- * returning to the interrupted task.
- *
- * Keep it simple: clear DR6 immediately.
- */
- set_debugreg(0, 6);
+ debug_enter(&dr6, &dr7);

- /* Filter out all the reserved bits which are preset to 1 */
- dr6 &= ~DR6_RESERVED;
+ nmi_enter();

/*
* The SDM says "The processor clears the BTF flag when it
@@ -786,7 +822,7 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)
#endif

if (notify_die(DIE_DEBUG, "debug", regs, (long)&dr6, error_code,
- SIGTRAP) == NOTIFY_STOP)
+ SIGTRAP) == NOTIFY_STOP)
goto exit;

/*
@@ -825,6 +861,7 @@ dotraplinkage void do_debug(struct pt_regs *regs, long error_code)

exit:
nmi_exit();
+ debug_exit(dr7);
}
NOKPROBE_SYMBOL(do_debug);