| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20251214183827.4z6nrrol4vz2tc5w@desk>
Date: Sun, 14 Dec 2025 10:38:27 -0800
From: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Nikolay Borisov <nik.borisov@...e.com>, x86@...nel.org,
David Kaplan <david.kaplan@....com>,
"H. Peter Anvin" <hpa@...or.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
Asit Mallick <asit.k.mallick@...el.com>,
Tao Zhang <tao1.zhang@...el.com>
Subject: Re: [PATCH v6 2/9] x86/bhi: Make clear_bhb_loop() effective on newer
CPUs
On Wed, Dec 10, 2025 at 01:35:42PM +0000, David Laight wrote:
> On Wed, 10 Dec 2025 14:31:31 +0200
> Nikolay Borisov <nik.borisov@...e.com> wrote:
>
> > On 2.12.25 г. 8:19 ч., Pawan Gupta wrote:
> > > As a mitigation for BHI, clear_bhb_loop() executes branches that overwrites
> > > the Branch History Buffer (BHB). On Alder Lake and newer parts this
> > > sequence is not sufficient because it doesn't clear enough entries. This
> > > was not an issue because these CPUs have a hardware control (BHI_DIS_S)
> > > that mitigates BHI in kernel.
> > >
> > > BHI variant of VMSCAPE requires isolating branch history between guests and
> > > userspace. Note that there is no equivalent hardware control for userspace.
> > > To effectively isolate branch history on newer CPUs, clear_bhb_loop()
> > > should execute sufficient number of branches to clear a larger BHB.
> > >
> > > Dynamically set the loop count of clear_bhb_loop() such that it is
> > > effective on newer CPUs too. Use the hardware control enumeration
> > > X86_FEATURE_BHI_CTRL to select the appropriate loop count.
> > >
> > > Suggested-by: Dave Hansen <dave.hansen@...ux.intel.com>
> > > Reviewed-by: Nikolay Borisov <nik.borisov@...e.com>
> > > Signed-off-by: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
> >
> > nit: My RB tag is incorrect, while I did agree with Dave's suggestion to
> > have global variables for the loop counts I haven't' really seen the
> > code so I couldn't have given my RB on something which I haven't seen
> > but did agree with in principle.
>
> I thought the plan was to use global variables rather than ALTERNATIVE.
> The performance of this code is dominated by the loop.
Using globals was much more involved, requiring changes in atleast 3 files.
The current ALTERNATIVE approach is much simpler and avoids additional
handling to make sure that globals are set correctly for all mitigation
modes of BHI and VMSCAPE.
[ BTW, I am travelling on a vacation and will be intermittently checking my
emails. ]
> I also found this code in arch/x86/net/bpf_jit_comp.c:
> if (cpu_feature_enabled(X86_FEATURE_CLEAR_BHB_LOOP)) {
> /* The clearing sequence clobbers eax and ecx. */
> EMIT1(0x50); /* push rax */
> EMIT1(0x51); /* push rcx */
> ip += 2;
>
> func = (u8 *)clear_bhb_loop;
> ip += x86_call_depth_emit_accounting(&prog, func, ip);
>
> if (emit_call(&prog, func, ip))
> return -EINVAL;
> EMIT1(0x59); /* pop rcx */
> EMIT1(0x58); /* pop rax */
> }
> which appears to assume that only rax and rcx are changed.
> Since all the counts are small, there is nothing stopping the code
> using the 8-bit registers %al, %ah, %cl and %ch.
Thanks for catching this.
> There are probably some schemes that only need one register.
> eg two separate ALTERNATIVE blocks.
Also, I think it is better to use a callee-saved register like rbx to avoid
callers having to save/restore registers. Something like below:
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9f6f4a7c5baf..ca4a34ce314a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1535,11 +1535,12 @@ SYM_CODE_END(rewind_stack_and_make_dead)
SYM_FUNC_START(clear_bhb_loop)
ANNOTATE_NOENDBR
push %rbp
+ push %rbx
mov %rsp, %rbp
/* loop count differs based on BHI_CTRL, see Intel's BHI guidance */
- ALTERNATIVE "movl $5, %ecx; movl $5, %edx", \
- "movl $12, %ecx; movl $7, %edx", X86_FEATURE_BHI_CTRL
+ ALTERNATIVE "movb $5, %bl", \
+ "movb $12, %bl", X86_FEATURE_BHI_CTRL
ANNOTATE_INTRA_FUNCTION_CALL
call 1f
@@ -1561,15 +1562,17 @@ SYM_FUNC_START(clear_bhb_loop)
* but some Clang versions (e.g. 18) don't like this.
*/
.skip 32 - 18, 0xcc
-2: movl %edx, %eax
+2: ALTERNATIVE "movb $5, %bh", \
+ "movb $7, %bh", X86_FEATURE_BHI_CTRL
3: jmp 4f
nop
-4: sub $1, %eax
+4: sub $1, %bh
jnz 3b
- sub $1, %ecx
+ sub $1, %bl
jnz 1b
.Lret2: RET
5:
+ pop %rbx
pop %rbp
RET
SYM_FUNC_END(clear_bhb_loop)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index c1ec14c55911..823b3f613774 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1593,11 +1593,6 @@ static int emit_spectre_bhb_barrier(u8 **pprog, u8 *ip,
u8 *func;
if (cpu_feature_enabled(X86_FEATURE_CLEAR_BHB_LOOP)) {
- /* The clearing sequence clobbers eax and ecx. */
- EMIT1(0x50); /* push rax */
- EMIT1(0x51); /* push rcx */
- ip += 2;
-
func = (u8 *)clear_bhb_loop;
ip += x86_call_depth_emit_accounting(&prog, func, ip);
@@ -1605,8 +1600,6 @@ static int emit_spectre_bhb_barrier(u8 **pprog, u8 *ip,
return -EINVAL;
/* Don't speculate past this until BHB is cleared */
EMIT_LFENCE();
- EMIT1(0x59); /* pop rcx */
- EMIT1(0x58); /* pop rax */
}
/* Insert IBHF instruction */
if ((cpu_feature_enabled(X86_FEATURE_CLEAR_BHB_LOOP) &&
Powered by blists - more mailing lists