2024-05-24 07:05:42

by Alexandre Chartre

[permalink] [raw]
Subject: [PATCH v2] x86/bhi: BHI mitigation can trigger warning in #DB handler

When BHI mitigation is enabled, if sysenter is invoked with the TF flag
set then entry_SYSENTER_compat uses CLEAR_BRANCH_HISTORY and calls the
clear_bhb_loop() before the TF flag is cleared. This causes the #DB
handler (exc_debug_kernel) to issue a warning because single-step is
used outside the entry_SYSENTER_compat function.

To address this issue, entry_SYSENTER_compat() should use
CLEAR_BRANCH_HISTORY after making sure flag the TF flag is cleared.

The problem can be reproduced with the following sequence:

$ cat sysenter_step.c
int main()
{ asm("pushf; pop %ax; bts $8,%ax; push %ax; popf; sysenter"); }

$ gcc -o sysenter_step sysenter_step.c

$ ./sysenter_step
Segmentation fault (core dumped)

The program is expected to crash, and the #DB handler will issue a warning.

Kernel log:

WARNING: CPU: 27 PID: 7000 at arch/x86/kernel/traps.c:1009 exc_debug_kernel+0xd2/0x160
...
RIP: 0010:exc_debug_kernel+0xd2/0x160
...
Call Trace:
<#DB>
? show_regs+0x68/0x80
? __warn+0x8c/0x140
? exc_debug_kernel+0xd2/0x160
? report_bug+0x175/0x1a0
? handle_bug+0x44/0x90
? exc_invalid_op+0x1c/0x70
? asm_exc_invalid_op+0x1f/0x30
? exc_debug_kernel+0xd2/0x160
exc_debug+0x43/0x50
asm_exc_debug+0x1e/0x40
RIP: 0010:clear_bhb_loop+0x0/0xb0
...
</#DB>
<TASK>
? entry_SYSENTER_compat_after_hwframe+0x6e/0x8d
</TASK>

Fixes: 7390db8aea0d ("x86/bhi: Add support for clearing branch history at syscall entry")
Reported-by: Suman Maity <[email protected]>
Signed-off-by: Alexandre Chartre <[email protected]>
---
arch/x86/entry/entry_64_compat.S | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 11c9b8efdc4c..ed0a5f2dc129 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -89,10 +89,6 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)

cld

- IBRS_ENTER
- UNTRAIN_RET
- CLEAR_BRANCH_HISTORY
-
/*
* SYSENTER doesn't filter flags, so we need to clear NT and AC
* ourselves. To save a few cycles, we can check whether
@@ -116,6 +112,16 @@ SYM_INNER_LABEL(entry_SYSENTER_compat_after_hwframe, SYM_L_GLOBAL)
jnz .Lsysenter_fix_flags
.Lsysenter_flags_fixed:

+ /*
+ * CPU bugs mitigations mechanisms can call other functions. They
+ * should be invoked after making sure TF is cleared because
+ * single-step is ignored only for instructions inside the
+ * entry_SYSENTER_compat function.
+ */
+ IBRS_ENTER
+ UNTRAIN_RET
+ CLEAR_BRANCH_HISTORY
+
movq %rsp, %rdi
call do_SYSENTER_32
jmp sysret32_from_system_call
--
2.39.3



2024-05-24 17:18:18

by Andrew Cooper

[permalink] [raw]
Subject: Re: [PATCH v2] x86/bhi: BHI mitigation can trigger warning in #DB handler

On 24/05/2024 8:04 am, Alexandre Chartre wrote:
> When BHI mitigation is enabled, if sysenter is invoked with the TF flag
> set then entry_SYSENTER_compat uses CLEAR_BRANCH_HISTORY and calls the
> clear_bhb_loop() before the TF flag is cleared. This causes the #DB
> handler (exc_debug_kernel) to issue a warning because single-step is
> used outside the entry_SYSENTER_compat function.
>
> To address this issue, entry_SYSENTER_compat() should use
> CLEAR_BRANCH_HISTORY after making sure flag the TF flag is cleared.
>
> The problem can be reproduced with the following sequence:
>
> $ cat sysenter_step.c
> int main()
> { asm("pushf; pop %ax; bts $8,%ax; push %ax; popf; sysenter"); }
>
> $ gcc -o sysenter_step sysenter_step.c
>
> $ ./sysenter_step
> Segmentation fault (core dumped)
>
> The program is expected to crash, and the #DB handler will issue a warning.
>
> Kernel log:
>
> WARNING: CPU: 27 PID: 7000 at arch/x86/kernel/traps.c:1009 exc_debug_kernel+0xd2/0x160
> ...
> RIP: 0010:exc_debug_kernel+0xd2/0x160
> ...
> Call Trace:
> <#DB>
> ? show_regs+0x68/0x80
> ? __warn+0x8c/0x140
> ? exc_debug_kernel+0xd2/0x160
> ? report_bug+0x175/0x1a0
> ? handle_bug+0x44/0x90
> ? exc_invalid_op+0x1c/0x70
> ? asm_exc_invalid_op+0x1f/0x30
> ? exc_debug_kernel+0xd2/0x160
> exc_debug+0x43/0x50
> asm_exc_debug+0x1e/0x40
> RIP: 0010:clear_bhb_loop+0x0/0xb0
> ...
> </#DB>
> <TASK>
> ? entry_SYSENTER_compat_after_hwframe+0x6e/0x8d
> </TASK>
>
> Fixes: 7390db8aea0d ("x86/bhi: Add support for clearing branch history at syscall entry")
> Reported-by: Suman Maity <[email protected]>
> Signed-off-by: Alexandre Chartre <[email protected]>

Reviewed-by: Andrew Cooper <[email protected]>

2024-05-24 20:42:30

by Pawan Gupta

[permalink] [raw]
Subject: Re: [PATCH v2] x86/bhi: BHI mitigation can trigger warning in #DB handler

On Fri, May 24, 2024 at 09:04:59AM +0200, Alexandre Chartre wrote:
> When BHI mitigation is enabled, if sysenter is invoked with the TF flag
> set then entry_SYSENTER_compat uses CLEAR_BRANCH_HISTORY and calls the
> clear_bhb_loop() before the TF flag is cleared. This causes the #DB
> handler (exc_debug_kernel) to issue a warning because single-step is
> used outside the entry_SYSENTER_compat function.
>
> To address this issue, entry_SYSENTER_compat() should use
> CLEAR_BRANCH_HISTORY after making sure flag the TF flag is cleared.
>
> The problem can be reproduced with the following sequence:
>
> $ cat sysenter_step.c
> int main()
> { asm("pushf; pop %ax; bts $8,%ax; push %ax; popf; sysenter"); }
>
> $ gcc -o sysenter_step sysenter_step.c
>
> $ ./sysenter_step
> Segmentation fault (core dumped)
>
> The program is expected to crash, and the #DB handler will issue a warning.
>
> Kernel log:
>
> WARNING: CPU: 27 PID: 7000 at arch/x86/kernel/traps.c:1009 exc_debug_kernel+0xd2/0x160
> ...
> RIP: 0010:exc_debug_kernel+0xd2/0x160
> ...
> Call Trace:
> <#DB>
> ? show_regs+0x68/0x80
> ? __warn+0x8c/0x140
> ? exc_debug_kernel+0xd2/0x160
> ? report_bug+0x175/0x1a0
> ? handle_bug+0x44/0x90
> ? exc_invalid_op+0x1c/0x70
> ? asm_exc_invalid_op+0x1f/0x30
> ? exc_debug_kernel+0xd2/0x160
> exc_debug+0x43/0x50
> asm_exc_debug+0x1e/0x40
> RIP: 0010:clear_bhb_loop+0x0/0xb0
> ...
> </#DB>
> <TASK>
> ? entry_SYSENTER_compat_after_hwframe+0x6e/0x8d
> </TASK>
>
> Fixes: 7390db8aea0d ("x86/bhi: Add support for clearing branch history at syscall entry")
> Reported-by: Suman Maity <[email protected]>
> Signed-off-by: Alexandre Chartre <[email protected]>

Reviewed-by: Pawan Gupta <[email protected]>