2020-02-25 23:30:38

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

From: Andy Lutomirski <[email protected]>

do_machine_check() can be raised in almost any context including the most
fragile ones. Prevent kprobes and tracing.

Signed-off-by: Andy Lutomirski <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/traps.h | 3 ---
arch/x86/kernel/cpu/mce/core.c | 12 ++++++++++--
2 files changed, 10 insertions(+), 5 deletions(-)

--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -88,9 +88,6 @@ dotraplinkage void do_page_fault(struct
dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *regs, long error_code);
dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code);
dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code);
-#ifdef CONFIG_X86_MCE
-dotraplinkage void do_machine_check(struct pt_regs *regs, long error_code);
-#endif
dotraplinkage void do_simd_coprocessor_error(struct pt_regs *regs, long error_code);
#ifdef CONFIG_X86_32
dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code);
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1213,8 +1213,14 @@ static void __mc_scan_banks(struct mce *
* On Intel systems this is entered on all CPUs in parallel through
* MCE broadcast. However some CPUs might be broken beyond repair,
* so be always careful when synchronizing with others.
+ *
+ * Tracing and kprobes are disabled: if we interrupted a kernel context
+ * with IF=1, we need to minimize stack usage. There are also recursion
+ * issues: if the machine check was due to a failure of the memory
+ * backing the user stack, tracing that reads the user stack will cause
+ * potentially infinite recursion.
*/
-void do_machine_check(struct pt_regs *regs, long error_code)
+void notrace do_machine_check(struct pt_regs *regs, long error_code)
{
DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
DECLARE_BITMAP(toclear, MAX_NR_BANKS);
@@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
ist_exit(regs);
}
EXPORT_SYMBOL_GPL(do_machine_check);
+NOKPROBE_SYMBOL(do_machine_check);

#ifndef CONFIG_MEMORY_FAILURE
int memory_failure(unsigned long pfn, int flags)
@@ -1892,10 +1899,11 @@ static void unexpected_machine_check(str
void (*machine_check_vector)(struct pt_regs *, long error_code) =
unexpected_machine_check;

-dotraplinkage void do_mce(struct pt_regs *regs, long error_code)
+dotraplinkage notrace void do_mce(struct pt_regs *regs, long error_code)
{
machine_check_vector(regs, error_code);
}
+NOKPROBE_SYMBOL(do_mce);

/*
* Called for each booted CPU to set up machine checks.


2020-02-26 01:15:53

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Tue, Feb 25, 2020 at 10:36:38PM +0100, Thomas Gleixner wrote:
> From: Andy Lutomirski <[email protected]>
>
> do_machine_check() can be raised in almost any context including the most
> fragile ones. Prevent kprobes and tracing.
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/include/asm/traps.h | 3 ---
> arch/x86/kernel/cpu/mce/core.c | 12 ++++++++++--
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -88,9 +88,6 @@ dotraplinkage void do_page_fault(struct
> dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *regs, long error_code);
> dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code);
> dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code);
> -#ifdef CONFIG_X86_MCE
> -dotraplinkage void do_machine_check(struct pt_regs *regs, long error_code);
> -#endif
> dotraplinkage void do_simd_coprocessor_error(struct pt_regs *regs, long error_code);
> #ifdef CONFIG_X86_32
> dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code);
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1213,8 +1213,14 @@ static void __mc_scan_banks(struct mce *
> * On Intel systems this is entered on all CPUs in parallel through
> * MCE broadcast. However some CPUs might be broken beyond repair,
> * so be always careful when synchronizing with others.
> + *
> + * Tracing and kprobes are disabled: if we interrupted a kernel context
> + * with IF=1, we need to minimize stack usage. There are also recursion
> + * issues: if the machine check was due to a failure of the memory
> + * backing the user stack, tracing that reads the user stack will cause
> + * potentially infinite recursion.
> */
> -void do_machine_check(struct pt_regs *regs, long error_code)
> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
> {
> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
> ist_exit(regs);
> }
> EXPORT_SYMBOL_GPL(do_machine_check);
> +NOKPROBE_SYMBOL(do_machine_check);

That won't protect all the function called by do_machine_check(), right?
There are lots of them.

2020-02-26 05:29:40

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On 2/25/20 5:13 PM, Frederic Weisbecker wrote:
> On Tue, Feb 25, 2020 at 10:36:38PM +0100, Thomas Gleixner wrote:
>> From: Andy Lutomirski <[email protected]>
>>
>> do_machine_check() can be raised in almost any context including the most
>> fragile ones. Prevent kprobes and tracing.
>>
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> Signed-off-by: Thomas Gleixner <[email protected]>
>> ---
>> arch/x86/include/asm/traps.h | 3 ---
>> arch/x86/kernel/cpu/mce/core.c | 12 ++++++++++--
>> 2 files changed, 10 insertions(+), 5 deletions(-)
>>
>> --- a/arch/x86/include/asm/traps.h
>> +++ b/arch/x86/include/asm/traps.h
>> @@ -88,9 +88,6 @@ dotraplinkage void do_page_fault(struct
>> dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *regs, long error_code);
>> dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code);
>> dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code);
>> -#ifdef CONFIG_X86_MCE
>> -dotraplinkage void do_machine_check(struct pt_regs *regs, long error_code);
>> -#endif
>> dotraplinkage void do_simd_coprocessor_error(struct pt_regs *regs, long error_code);
>> #ifdef CONFIG_X86_32
>> dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code);
>> --- a/arch/x86/kernel/cpu/mce/core.c
>> +++ b/arch/x86/kernel/cpu/mce/core.c
>> @@ -1213,8 +1213,14 @@ static void __mc_scan_banks(struct mce *
>> * On Intel systems this is entered on all CPUs in parallel through
>> * MCE broadcast. However some CPUs might be broken beyond repair,
>> * so be always careful when synchronizing with others.
>> + *
>> + * Tracing and kprobes are disabled: if we interrupted a kernel context
>> + * with IF=1, we need to minimize stack usage. There are also recursion
>> + * issues: if the machine check was due to a failure of the memory
>> + * backing the user stack, tracing that reads the user stack will cause
>> + * potentially infinite recursion.
>> */
>> -void do_machine_check(struct pt_regs *regs, long error_code)
>> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
>> {
>> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
>> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
>> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
>> ist_exit(regs);
>> }
>> EXPORT_SYMBOL_GPL(do_machine_check);
>> +NOKPROBE_SYMBOL(do_machine_check);
>
> That won't protect all the function called by do_machine_check(), right?
> There are lots of them.
>

It at least means we can survive to run actual C code in
do_machine_check(), which lets us try to mitigate this issue further.
PeterZ has patches for that, and maybe this series fixes it later on.
(I'm reading in order!)

2020-02-26 11:18:55

by Borislav Petkov

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Tue, Feb 25, 2020 at 10:36:38PM +0100, Thomas Gleixner wrote:
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1213,8 +1213,14 @@ static void __mc_scan_banks(struct mce *
> * On Intel systems this is entered on all CPUs in parallel through
> * MCE broadcast. However some CPUs might be broken beyond repair,
> * so be always careful when synchronizing with others.
> + *
> + * Tracing and kprobes are disabled: if we interrupted a kernel context
> + * with IF=1, we need to minimize stack usage. There are also recursion
> + * issues: if the machine check was due to a failure of the memory
> + * backing the user stack, tracing that reads the user stack will cause
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Had to read this a couple of times to parse that formulation properly.
Make that

"... backing the user stack, tracing code which accesses same user stack
will potentially cause an infinite recursion."

With that:

Reviewed-by: Borislav Petkov <[email protected]>

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-02-26 13:29:42

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Tue, Feb 25, 2020 at 09:29:00PM -0800, Andy Lutomirski wrote:

> >> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
> >> {
> >> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
> >> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
> >> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
> >> ist_exit(regs);
> >> }
> >> EXPORT_SYMBOL_GPL(do_machine_check);
> >> +NOKPROBE_SYMBOL(do_machine_check);
> >
> > That won't protect all the function called by do_machine_check(), right?
> > There are lots of them.
> >
>
> It at least means we can survive to run actual C code in
> do_machine_check(), which lets us try to mitigate this issue further.
> PeterZ has patches for that, and maybe this series fixes it later on.
> (I'm reading in order!)

Yeah, I don't cover that either. Making the kernel completely kprobe
safe is _lots_ more work I think.

We really need some form of automation for this :/ The current situation
is completely nonsatisfactory.

2020-02-26 15:11:08

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Wed, Feb 26, 2020 at 5:28 AM Peter Zijlstra <[email protected]> wrote:
>
> On Tue, Feb 25, 2020 at 09:29:00PM -0800, Andy Lutomirski wrote:
>
> > >> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
> > >> {
> > >> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
> > >> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
> > >> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
> > >> ist_exit(regs);
> > >> }
> > >> EXPORT_SYMBOL_GPL(do_machine_check);
> > >> +NOKPROBE_SYMBOL(do_machine_check);
> > >
> > > That won't protect all the function called by do_machine_check(), right?
> > > There are lots of them.
> > >
> >
> > It at least means we can survive to run actual C code in
> > do_machine_check(), which lets us try to mitigate this issue further.
> > PeterZ has patches for that, and maybe this series fixes it later on.
> > (I'm reading in order!)
>
> Yeah, I don't cover that either. Making the kernel completely kprobe
> safe is _lots_ more work I think.
>
> We really need some form of automation for this :/ The current situation
> is completely nonsatisfactory.

I've looked at too many patches lately and lost track a bit of which
is which. Shouldn't a simple tracing_disable() or similar in
do_machine_check() be sufficient? We'd maybe want automation to check
everything before it. We still need to survive hitting a kprobe int3,
but that shouldn't have recursion issues.

(Yes, that function doesn't exist in current kernels. And we'd need
to make sure that BPF respects it.)

2020-02-26 16:18:15

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Wed, Feb 26, 2020 at 07:10:01AM -0800, Andy Lutomirski wrote:
> On Wed, Feb 26, 2020 at 5:28 AM Peter Zijlstra <[email protected]> wrote:
> > On Tue, Feb 25, 2020 at 09:29:00PM -0800, Andy Lutomirski wrote:
> >
> > > >> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
> > > >> {
> > > >> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
> > > >> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
> > > >> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
> > > >> ist_exit(regs);
> > > >> }
> > > >> EXPORT_SYMBOL_GPL(do_machine_check);
> > > >> +NOKPROBE_SYMBOL(do_machine_check);
> > > >
> > > > That won't protect all the function called by do_machine_check(), right?
> > > > There are lots of them.
> > > >
> > >
> > > It at least means we can survive to run actual C code in
> > > do_machine_check(), which lets us try to mitigate this issue further.
> > > PeterZ has patches for that, and maybe this series fixes it later on.
> > > (I'm reading in order!)
> >
> > Yeah, I don't cover that either. Making the kernel completely kprobe
> > safe is _lots_ more work I think.
> >
> > We really need some form of automation for this :/ The current situation
> > is completely nonsatisfactory.
>
> I've looked at too many patches lately and lost track a bit of which
> is which. Shouldn't a simple tracing_disable() or similar in
> do_machine_check() be sufficient?

It entirely depends on what the goal is :-/ On the one hand I see why
people might want function tracing / kprobes enabled, OTOH it's all
mighty frigging scary. Any tracing/probing/whatever on an MCE has the
potential to make a bad situation worse -- not unlike the same on #DF.

The same with that compiler instrumentation crap; allowing kprobes on
*SAN code has the potential to inject probes in arbitrary random code.
At the same time, if you're running a kernel with that on and injecting
kprobes in it, you're welcome to own the remaining pieces.

How far do we want to go? At some point I think we'll have to give
people rope, show then the knot and leave them be.

> We'd maybe want automation to check
> everything before it. We still need to survive hitting a kprobe int3,
> but that shouldn't have recursion issues.

Right, so I think avoiding the obvious recursion issues is a more
tractable problem and yes some 'safe' spot annotation should be enough
to get automation working for that -- mostly.

2020-02-26 17:29:21

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()



> On Feb 26, 2020, at 8:08 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Feb 26, 2020 at 07:10:01AM -0800, Andy Lutomirski wrote:
>>> On Wed, Feb 26, 2020 at 5:28 AM Peter Zijlstra <[email protected]> wrote:
>>>> On Tue, Feb 25, 2020 at 09:29:00PM -0800, Andy Lutomirski wrote:
>>>
>>>>>> +void notrace do_machine_check(struct pt_regs *regs, long error_code)
>>>>>> {
>>>>>> DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
>>>>>> DECLARE_BITMAP(toclear, MAX_NR_BANKS);
>>>>>> @@ -1360,6 +1366,7 @@ void do_machine_check(struct pt_regs *re
>>>>>> ist_exit(regs);
>>>>>> }
>>>>>> EXPORT_SYMBOL_GPL(do_machine_check);
>>>>>> +NOKPROBE_SYMBOL(do_machine_check);
>>>>>
>>>>> That won't protect all the function called by do_machine_check(), right?
>>>>> There are lots of them.
>>>>>
>>>>
>>>> It at least means we can survive to run actual C code in
>>>> do_machine_check(), which lets us try to mitigate this issue further.
>>>> PeterZ has patches for that, and maybe this series fixes it later on.
>>>> (I'm reading in order!)
>>>
>>> Yeah, I don't cover that either. Making the kernel completely kprobe
>>> safe is _lots_ more work I think.
>>>
>>> We really need some form of automation for this :/ The current situation
>>> is completely nonsatisfactory.
>>
>> I've looked at too many patches lately and lost track a bit of which
>> is which. Shouldn't a simple tracing_disable() or similar in
>> do_machine_check() be sufficient?
>
> It entirely depends on what the goal is :-/ On the one hand I see why
> people might want function tracing / kprobes enabled, OTOH it's all
> mighty frigging scary. Any tracing/probing/whatever on an MCE has the
> potential to make a bad situation worse -- not unlike the same on #DF.
>
> The same with that compiler instrumentation crap; allowing kprobes on
> *SAN code has the potential to inject probes in arbitrary random code.
> At the same time, if you're running a kernel with that on and injecting
> kprobes in it, you're welcome to own the remaining pieces.
>

Agreed.

> How far do we want to go? At some point I think we'll have to give
> people rope, show then the knot and leave them be.

If someone puts a kprobe on some TLB flush thing and an MCE does memory failure handling, it would be polite to avoid crashing. OTOH the x86 memory failure story is *so* bad that I’m not sure how well we can ever really expect this to work.

I think we should aim to get the entry part correct, and if the meat of the function explodes, so be it.

>
>> We'd maybe want automation to check
>> everything before it. We still need to survive hitting a kprobe int3,
>> but that shouldn't have recursion issues.
>
> Right, so I think avoiding the obvious recursion issues is a more
> tractable problem and yes some 'safe' spot annotation should be enough
> to get automation working for that -- mostly.

2020-02-26 18:43:05

by Borislav Petkov

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Wed, Feb 26, 2020 at 09:28:51AM -0800, Andy Lutomirski wrote:
> > It entirely depends on what the goal is :-/ On the one hand I see why
> > people might want function tracing / kprobes enabled, OTOH it's all
> > mighty frigging scary. Any tracing/probing/whatever on an MCE has the
> > potential to make a bad situation worse -- not unlike the same on #DF.

FWIW, I had this at the beginning of the #MC handler in a feeble attempt
to poke at this:

+ hw_breakpoint_disable();
+ static_key_disable(&__tracepoint_read_msr.key);
+ tracing_off();

But then Tony noted that some recoverable errors do get reported with an
#MC exception so we would have to look at the error severity and then
decide whether to allow tracing or not.

But the error severity happens all the way down in __mc_scan_banks() -
i.e., we've executed the half handler already.

So, frankly, I wanna say, f*ck tracing etc - there are certain handlers
which simply don't allow it. And we'll only consider changing that when
a really good reason for it appears...

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-02-26 19:01:44

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Wed, Feb 26, 2020 at 07:42:37PM +0100, Borislav Petkov wrote:
> On Wed, Feb 26, 2020 at 09:28:51AM -0800, Andy Lutomirski wrote:
> > > It entirely depends on what the goal is :-/ On the one hand I see why
> > > people might want function tracing / kprobes enabled, OTOH it's all
> > > mighty frigging scary. Any tracing/probing/whatever on an MCE has the
> > > potential to make a bad situation worse -- not unlike the same on #DF.
>
> FWIW, I had this at the beginning of the #MC handler in a feeble attempt
> to poke at this:
>
> + hw_breakpoint_disable();
> + static_key_disable(&__tracepoint_read_msr.key);
> + tracing_off();

You can't do static_key_disable() from an IST

2020-02-26 19:09:43

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()



> On Feb 26, 2020, at 10:59 AM, Peter Zijlstra <[email protected]> wrote:
>
> On Wed, Feb 26, 2020 at 07:42:37PM +0100, Borislav Petkov wrote:
>> On Wed, Feb 26, 2020 at 09:28:51AM -0800, Andy Lutomirski wrote:
>>>> It entirely depends on what the goal is :-/ On the one hand I see why
>>>> people might want function tracing / kprobes enabled, OTOH it's all
>>>> mighty frigging scary. Any tracing/probing/whatever on an MCE has the
>>>> potential to make a bad situation worse -- not unlike the same on #DF.
>>
>> FWIW, I had this at the beginning of the #MC handler in a feeble attempt
>> to poke at this:
>>
>> + hw_breakpoint_disable();
>> + static_key_disable(&__tracepoint_read_msr.key);
>> + tracing_off();
>
> You can't do static_key_disable() from an IST

Can we set a percpu variable saying “in some stupid context, don’t trace”?

2020-02-26 21:01:32

by Steven Rostedt

[permalink] [raw]
Subject: Re: [patch 02/10] x86/mce: Disable tracing and kprobes on do_machine_check()

On Wed, 26 Feb 2020 11:09:03 -0800
Andy Lutomirski <[email protected]> wrote:

> > On Feb 26, 2020, at 10:59 AM, Peter Zijlstra <[email protected]> wrote:
> >
> > On Wed, Feb 26, 2020 at 07:42:37PM +0100, Borislav Petkov wrote:
> >> On Wed, Feb 26, 2020 at 09:28:51AM -0800, Andy Lutomirski wrote:
> >>>> It entirely depends on what the goal is :-/ On the one hand I see why
> >>>> people might want function tracing / kprobes enabled, OTOH it's all
> >>>> mighty frigging scary. Any tracing/probing/whatever on an MCE has the
> >>>> potential to make a bad situation worse -- not unlike the same on #DF.
> >>
> >> FWIW, I had this at the beginning of the #MC handler in a feeble attempt
> >> to poke at this:
> >>
> >> + hw_breakpoint_disable();
> >> + static_key_disable(&__tracepoint_read_msr.key);
> >> + tracing_off();
> >
> > You can't do static_key_disable() from an IST
>
> Can we set a percpu variable saying “in some stupid context, don’t trace”?

Matter's what kind of tracing you care about? "tracing_off()" only stops
writing to the ring buffer, but all the mechanisms are still in place (the
things you want to stop).

And "tracing" is not the issue. The issue is usually the breakpoint that is
added to modify the jump labels to enable tracing, or a flag that has a
trace event do a user space stack dump. It's not tracing you want to stop,
its the other parts that are attached to tracing that makes it dangerous.

-- Steve

Subject: [tip: x86/entry] x86/mce: Disable tracing and kprobes on do_machine_check()

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

Commit-ID: 55ba18d6ed37a28cf8b8ca79e9aef4cf98183bb7
Gitweb: https://git.kernel.org/tip/55ba18d6ed37a28cf8b8ca79e9aef4cf98183bb7
Author: Andy Lutomirski <[email protected]>
AuthorDate: Tue, 25 Feb 2020 22:36:38 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 27 Feb 2020 14:48:39 +01:00

x86/mce: Disable tracing and kprobes on do_machine_check()

do_machine_check() can be raised in almost any context including the most
fragile ones. Prevent kprobes and tracing.

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

---
arch/x86/include/asm/traps.h | 3 ---
arch/x86/kernel/cpu/mce/core.c | 12 ++++++++++--
2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
index ffa0dc8..e1c660b 100644
--- a/arch/x86/include/asm/traps.h
+++ b/arch/x86/include/asm/traps.h
@@ -88,9 +88,6 @@ dotraplinkage void do_page_fault(struct pt_regs *regs, unsigned long error_code,
dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *regs, long error_code);
dotraplinkage void do_coprocessor_error(struct pt_regs *regs, long error_code);
dotraplinkage void do_alignment_check(struct pt_regs *regs, long error_code);
-#ifdef CONFIG_X86_MCE
-dotraplinkage void do_machine_check(struct pt_regs *regs, long error_code);
-#endif
dotraplinkage void do_simd_coprocessor_error(struct pt_regs *regs, long error_code);
#ifdef CONFIG_X86_32
dotraplinkage void do_iret_error(struct pt_regs *regs, long error_code);
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 2c4f949..32ecc59 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1213,8 +1213,14 @@ static void __mc_scan_banks(struct mce *m, struct mce *final,
* On Intel systems this is entered on all CPUs in parallel through
* MCE broadcast. However some CPUs might be broken beyond repair,
* so be always careful when synchronizing with others.
+ *
+ * Tracing and kprobes are disabled: if we interrupted a kernel context
+ * with IF=1, we need to minimize stack usage. There are also recursion
+ * issues: if the machine check was due to a failure of the memory
+ * backing the user stack, tracing that reads the user stack will cause
+ * potentially infinite recursion.
*/
-void do_machine_check(struct pt_regs *regs, long error_code)
+void notrace do_machine_check(struct pt_regs *regs, long error_code)
{
DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
DECLARE_BITMAP(toclear, MAX_NR_BANKS);
@@ -1360,6 +1366,7 @@ out_ist:
ist_exit(regs);
}
EXPORT_SYMBOL_GPL(do_machine_check);
+NOKPROBE_SYMBOL(do_machine_check);

#ifndef CONFIG_MEMORY_FAILURE
int memory_failure(unsigned long pfn, int flags)
@@ -1892,10 +1899,11 @@ static void unexpected_machine_check(struct pt_regs *regs, long error_code)
void (*machine_check_vector)(struct pt_regs *, long error_code) =
unexpected_machine_check;

-dotraplinkage void do_mce(struct pt_regs *regs, long error_code)
+dotraplinkage notrace void do_mce(struct pt_regs *regs, long error_code)
{
machine_check_vector(regs, error_code);
}
+NOKPROBE_SYMBOL(do_mce);

/*
* Called for each booted CPU to set up machine checks.