[PATCH v7 03/10] x86/bhi: Rename clear_bhb_loop() to clear_bhb_loop_nofence()

Pawan Gupta posted 10 patches 2 weeks, 3 days ago
There is a newer version of this series
[PATCH v7 03/10] x86/bhi: Rename clear_bhb_loop() to clear_bhb_loop_nofence()
Posted by Pawan Gupta 2 weeks, 3 days ago
To reflect the recent change that moved LFENCE to the caller side.

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
---
 arch/x86/entry/entry_64.S            | 8 ++++----
 arch/x86/include/asm/nospec-branch.h | 6 +++---
 arch/x86/net/bpf_jit_comp.c          | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 8128e00ca73f..e9b81b95fcc8 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1532,7 +1532,7 @@ SYM_CODE_END(rewind_stack_and_make_dead)
  * Note, callers should use a speculation barrier like LFENCE immediately after
  * a call to this function to ensure BHB is cleared before indirect branches.
  */
-SYM_FUNC_START(clear_bhb_loop)
+SYM_FUNC_START(clear_bhb_loop_nofence)
 	ANNOTATE_NOENDBR
 	push	%rbp
 	/* BPF caller may require %rax to be preserved */
@@ -1579,6 +1579,6 @@ SYM_FUNC_START(clear_bhb_loop)
 	pop	%rax
 	pop	%rbp
 	RET
-SYM_FUNC_END(clear_bhb_loop)
-EXPORT_SYMBOL_FOR_KVM(clear_bhb_loop)
-STACK_FRAME_NON_STANDARD(clear_bhb_loop)
+SYM_FUNC_END(clear_bhb_loop_nofence)
+EXPORT_SYMBOL_FOR_KVM(clear_bhb_loop_nofence)
+STACK_FRAME_NON_STANDARD(clear_bhb_loop_nofence)
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 70b377fcbc1c..0f5e6ed6c9c2 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -331,11 +331,11 @@
 
 #ifdef CONFIG_X86_64
 .macro CLEAR_BRANCH_HISTORY
-	ALTERNATIVE "", "call clear_bhb_loop; lfence", X86_FEATURE_CLEAR_BHB_LOOP
+	ALTERNATIVE "", "call clear_bhb_loop_nofence; lfence", X86_FEATURE_CLEAR_BHB_LOOP
 .endm
 
 .macro CLEAR_BRANCH_HISTORY_VMEXIT
-	ALTERNATIVE "", "call clear_bhb_loop; lfence", X86_FEATURE_CLEAR_BHB_VMEXIT
+	ALTERNATIVE "", "call clear_bhb_loop_nofence; lfence", X86_FEATURE_CLEAR_BHB_VMEXIT
 .endm
 #else
 #define CLEAR_BRANCH_HISTORY
@@ -389,7 +389,7 @@ extern void entry_untrain_ret(void);
 extern void write_ibpb(void);
 
 #ifdef CONFIG_X86_64
-extern void clear_bhb_loop(void);
+extern void clear_bhb_loop_nofence(void);
 #endif
 
 extern void (*x86_return_thunk)(void);
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index e2cceabb23e8..b57e9ab51c5d 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1614,7 +1614,7 @@ static int emit_spectre_bhb_barrier(u8 **pprog, u8 *ip,
 	u8 *func;
 
 	if (cpu_feature_enabled(X86_FEATURE_CLEAR_BHB_LOOP)) {
-		func = (u8 *)clear_bhb_loop;
+		func = (u8 *)clear_bhb_loop_nofence;
 		ip += x86_call_depth_emit_accounting(&prog, func, ip);
 
 		if (emit_call(&prog, func, ip))

-- 
2.34.1
Re: [PATCH v7 03/10] x86/bhi: Rename clear_bhb_loop() to clear_bhb_loop_nofence()
Posted by Nikolay Borisov 1 week, 6 days ago

On 19.03.26 г. 17:40 ч., Pawan Gupta wrote:
> To reflect the recent change that moved LFENCE to the caller side.
> 
> Suggested-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>


Nit: I think having the _nofence in the function name is leaking an 
implementation detail into the name/interface. I.e things change and we 
decide that the implementation of a particular function must change so 
we just do the change and substantiate it in the commit message or in a 
comment. Especially that we don't have a "with an lfence" version.

What's more I'd consider this a "private" function, that's called via 
the CLEAR_BRANCH_HISTORY macros, the only place it's called directly is 
in the bpf jit code, but that's more of an exception.

Still,

Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

<snip>

Re: [PATCH v7 03/10] x86/bhi: Rename clear_bhb_loop() to clear_bhb_loop_nofence()
Posted by Pawan Gupta 1 week, 6 days ago
On Mon, Mar 23, 2026 at 04:44:24PM +0200, Nikolay Borisov wrote:
> 
> 
> On 19.03.26 г. 17:40 ч., Pawan Gupta wrote:
> > To reflect the recent change that moved LFENCE to the caller side.
> > 
> > Suggested-by: Borislav Petkov <bp@alien8.de>
> > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> 
> 
> Nit: I think having the _nofence in the function name is leaking an
> implementation detail into the name/interface. I.e things change and we
> decide that the implementation of a particular function must change so we
> just do the change and substantiate it in the commit message or in a
> comment. Especially that we don't have a "with an lfence" version.

The explicit "_nofence" is because the series changes the implementation of
clear_bhb_loop() from lfence. If new call sites miss to add an lfence when
it is required could lead to a security issue. Having the "_nofence" in the
name helps avoid it.

Apart from the name, the commit message of patch 1/10 and the comment in
clear_bhb_loop() implementation covers this.

> What's more I'd consider this a "private" function, that's called via the
> CLEAR_BRANCH_HISTORY macros, the only place it's called directly is in the
> bpf jit code, but that's more of an exception.

Another place where the explicit "_nofence" in the name could help is while
applying the mitigation in vmscape_apply_mitigation(), which sets the
static call:

vmscape_apply_mitigation()
{
...
    if (vmscape_mitigation == VMSCAPE_MITIGATION_IBPB_EXIT_TO_USER)
        static_call_update(vmscape_predictor_flush, write_ibpb);
    else if (vmscape_mitigation == VMSCAPE_MITIGATION_BHB_CLEAR_EXIT_TO_USER)
        static_call_update(vmscape_predictor_flush, clear_bhb_loop_nofence);

> Still,
> 
> Reviewed-by: Nikolay Borisov <nik.borisov@suse.com>

Thank you.