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

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

Posted by Alexandre Chartre 1 year, 8 months ago

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 <suman.m.maity@oracle.com>
Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
---
 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

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

Posted by Josh Poimboeuf 1 year, 7 months ago

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.

Reviewed-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh

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

Posted by Andrew Cooper 1 year, 8 months ago

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 <suman.m.maity@oracle.com>
> Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Posted by Pawan Gupta 1 year, 8 months ago

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 <suman.m.maity@oracle.com>
> Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>

Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>

[tip: x86/urgent] x86/bhi: Avoid warning in #DB handler due to BHI mitigation

Posted by tip-bot2 for Alexandre Chartre 1 year, 7 months ago

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

Commit-ID:     ac8b270b61d48fcc61f052097777e3b5e11591e0
Gitweb:        https://git.kernel.org/tip/ac8b270b61d48fcc61f052097777e3b5e11591e0
Author:        Alexandre Chartre <alexandre.chartre@oracle.com>
AuthorDate:    Fri, 24 May 2024 09:04:59 +02:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Wed, 03 Jul 2024 13:26:30 +02:00

x86/bhi: Avoid warning in #DB handler due to BHI mitigation

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 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>

  [ bp: Massage commit message. ]

Fixes: 7390db8aea0d ("x86/bhi: Add support for clearing branch history at syscall entry")
Reported-by: Suman Maity <suman.m.maity@oracle.com>
Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Reviewed-by: Josh Poimboeuf <jpoimboe@kernel.org>
Link: https://lore.kernel.org/r/20240524070459.3674025-1-alexandre.chartre@oracle.com
---
 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 11c9b8e..ed0a5f2 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