1. Patchew
  2. linux
  3. View series

[PATCH] x86/fred: Correct speculative safety in fred_extint()

Andrew Cooper posted 1 patch 1 month ago 
arch/x86/entry/entry_fred.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[PATCH] x86/fred: Correct speculative safety in fred_extint()
Posted by Andrew Cooper 1 month ago 
array_index_nospec() is no use if the result gets spilled to the stack, as
it makes the believed safe-under-speculation value subject to memory
predictions.

For all practical purposes, this means array_index_nospec() must be used in
the expression that accesses the array.

As the code currently stands, it's the wrong side of irqentry_enter(), and
'index' is put into %ebp across the function call.

Remove the index variable and reposition array_index_nospec(), so it's
calculated immediately before the array access.

Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Xin Li <xin@zytor.com>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@kernel.org>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: Borislav Petkov <bp@alien8.de>
CC: Dave Hansen <dave.hansen@linux.intel.com>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org

This is why we have array_access_nospec() in Xen, so you can't separate the
safety calculation from the array access.

The observant reader might notice that the result of reading sysvec_table[] is
also subject to memory predictions.  Aren't CPUs wonderful...

In practice, even having array_index_nospec() part of the array access
expression is no guarantee of avoiding spilling to the stack.  KASAN is liable
to hide a function call behind the scenes, while UBSAN is very good at
inserting it's own unsafe range checks around objects it knows the size of.
Aren't compilers wonderful...
---
 arch/x86/entry/entry_fred.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index 94e626cc6a07..4fc5b176d3ed 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -159,8 +159,6 @@ void __init fred_complete_exception_setup(void)
 static noinstr void fred_extint(struct pt_regs *regs)
 {
 	unsigned int vector = regs->fred_ss.vector;
-	unsigned int index = array_index_nospec(vector - FIRST_SYSTEM_VECTOR,
-						NR_SYSTEM_VECTORS);
 
 	if (WARN_ON_ONCE(vector < FIRST_EXTERNAL_VECTOR))
 		return;
@@ -169,7 +167,8 @@ static noinstr void fred_extint(struct pt_regs *regs)
 		irqentry_state_t state = irqentry_enter(regs);
 
 		instrumentation_begin();
-		sysvec_table[index](regs);
+		sysvec_table[array_index_nospec(vector - FIRST_SYSTEM_VECTOR,
+						NR_SYSTEM_VECTORS)](regs);
 		instrumentation_end();
 		irqentry_exit(regs, state);
 	} else {

base-commit: 7f98ab9da046865d57c102fd3ca9669a29845f67
-- 
2.39.5
Re: [PATCH] x86/fred: Correct speculative safety in fred_extint()
Posted by Peter Zijlstra 1 month ago 
On Tue, Jan 06, 2026 at 01:15:04PM +0000, Andrew Cooper wrote:
> array_index_nospec() is no use if the result gets spilled to the stack, as
> it makes the believed safe-under-speculation value subject to memory
> predictions.
> 
> For all practical purposes, this means array_index_nospec() must be used in
> the expression that accesses the array.
> 
> As the code currently stands, it's the wrong side of irqentry_enter(), and
> 'index' is put into %ebp across the function call.
> 
> Remove the index variable and reposition array_index_nospec(), so it's
> calculated immediately before the array access.
> 
> Fixes: 14619d912b65 ("x86/fred: FRED entry/exit and dispatch code")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

I suppose I can go stick this in x86/urgent or so.

> This is why we have array_access_nospec() in Xen, so you can't separate the
> safety calculation from the array access.
> 
> The observant reader might notice that the result of reading sysvec_table[] is
> also subject to memory predictions.  Aren't CPUs wonderful...
> 
> In practice, even having array_index_nospec() part of the array access
> expression is no guarantee of avoiding spilling to the stack.  KASAN is liable
> to hide a function call behind the scenes, while UBSAN is very good at
> inserting it's own unsafe range checks around objects it knows the size of.
> Aren't compilers wonderful...

Yeah, then again nobody should be running *SAN kernels in production,
right ;-)

> ---
>  arch/x86/entry/entry_fred.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index 94e626cc6a07..4fc5b176d3ed 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -159,8 +159,6 @@ void __init fred_complete_exception_setup(void)
>  static noinstr void fred_extint(struct pt_regs *regs)
>  {
>  	unsigned int vector = regs->fred_ss.vector;
> -	unsigned int index = array_index_nospec(vector - FIRST_SYSTEM_VECTOR,
> -						NR_SYSTEM_VECTORS);
>  
>  	if (WARN_ON_ONCE(vector < FIRST_EXTERNAL_VECTOR))
>  		return;
> @@ -169,7 +167,8 @@ static noinstr void fred_extint(struct pt_regs *regs)
>  		irqentry_state_t state = irqentry_enter(regs);
>  
>  		instrumentation_begin();
> -		sysvec_table[index](regs);
> +		sysvec_table[array_index_nospec(vector - FIRST_SYSTEM_VECTOR,
> +						NR_SYSTEM_VECTORS)](regs);
>  		instrumentation_end();
>  		irqentry_exit(regs, state);
>  	} else {
Re: [PATCH] x86/fred: Correct speculative safety in fred_extint()
Posted by Andrew Cooper 1 month ago 
On 06/01/2026 3:20 pm, Peter Zijlstra wrote:
> On Tue, Jan 06, 2026 at 01:15:04PM +0000, Andrew Cooper wrote:
>> This is why we have array_access_nospec() in Xen, so you can't separate the
>> safety calculation from the array access.
>>
>> The observant reader might notice that the result of reading sysvec_table[] is
>> also subject to memory predictions.  Aren't CPUs wonderful...
>>
>> In practice, even having array_index_nospec() part of the array access
>> expression is no guarantee of avoiding spilling to the stack.  KASAN is liable
>> to hide a function call behind the scenes, while UBSAN is very good at
>> inserting it's own unsafe range checks around objects it knows the size of.
>> Aren't compilers wonderful...
> Yeah, then again nobody should be running *SAN kernels in production,
> right ;-)

Aren't distros wonderful...

~Andrew
Re: [PATCH] x86/fred: Correct speculative safety in fred_extint()
Posted by H. Peter Anvin 1 month ago 
On January 6, 2026 8:46:41 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>On 06/01/2026 3:20 pm, Peter Zijlstra wrote:
>> On Tue, Jan 06, 2026 at 01:15:04PM +0000, Andrew Cooper wrote:
>>> This is why we have array_access_nospec() in Xen, so you can't separate the
>>> safety calculation from the array access.
>>>
>>> The observant reader might notice that the result of reading sysvec_table[] is
>>> also subject to memory predictions.  Aren't CPUs wonderful...
>>>
>>> In practice, even having array_index_nospec() part of the array access
>>> expression is no guarantee of avoiding spilling to the stack.  KASAN is liable
>>> to hide a function call behind the scenes, while UBSAN is very good at
>>> inserting it's own unsafe range checks around objects it knows the size of.
>>> Aren't compilers wonderful...
>> Yeah, then again nobody should be running *SAN kernels in production,
>> right ;-)
>
>Aren't distros wonderful...
>
>~Andrew

This would probably be best addressed with an intrinsic. 

That being said, there are a few reasons why this may not matter in the case of FRED specifically. 

Note that this value comes from memory in the first place, *but* it comes from memory just written to the stack and thus is virtually guaranteed to be live in the L1 TLB and cache. Now with PTL about to be released, I'm also actively looking at ways to optimize FRED entry. One way is to precompute certain useful values in the entry path, instead of simply zeroing the GPRs one can extract data that will be needed further on in the data path into the argument registers. 

Note that FRED entry is an architectural speculation barrier.

Furthermore, my measurements so far indicate that doing an early-out on the SYSCALL path is pretty essential. It might even be valuable enough to do in the assembly stub code.
Re: [PATCH] x86/fred: Correct speculative safety in fred_extint()
Posted by Andrew Cooper 1 month ago 
On 06/01/2026 5:06 pm, H. Peter Anvin wrote:
> On January 6, 2026 8:46:41 AM PST, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 06/01/2026 3:20 pm, Peter Zijlstra wrote:
>>> On Tue, Jan 06, 2026 at 01:15:04PM +0000, Andrew Cooper wrote:
>>>> This is why we have array_access_nospec() in Xen, so you can't separate the
>>>> safety calculation from the array access.
>>>>
>>>> The observant reader might notice that the result of reading sysvec_table[] is
>>>> also subject to memory predictions.  Aren't CPUs wonderful...
>>>>
>>>> In practice, even having array_index_nospec() part of the array access
>>>> expression is no guarantee of avoiding spilling to the stack.  KASAN is liable
>>>> to hide a function call behind the scenes, while UBSAN is very good at
>>>> inserting it's own unsafe range checks around objects it knows the size of.
>>>> Aren't compilers wonderful...
>>> Yeah, then again nobody should be running *SAN kernels in production,
>>> right ;-)
>> Aren't distros wonderful...
>>
>> ~Andrew
> This would probably be best addressed with an intrinsic.

It's called __builtin_speculation_safe_value() and it's as useful as a
chocolate teapot.  When trying to report code generation bugs against
it, I discovered that GCC's security policy is "erm?  maybe try talking
to SUSE" and LLVM was less developed than that.  I gave up trying to get
it adjusted, and am yet to find a user of it outside of the compiler
selftests.

> That being said, there are a few reasons why this may not matter in the case of FRED specifically. 
>
> Note that this value comes from memory in the first place, *but* it comes from memory just written to the stack and thus is virtually guaranteed to be live in the L1 TLB and cache.

All true, but not really relevant.

The real data being hot still doesn't guarantee that memory prediction
is going to guess the right address to speculatively load.

I'm aware that training the memory predictor is far harder than training
the branch predictor, but it possible, and in this case it renders-moot
the attempt to make a safe-even-under-speculation array index.

> Now with PTL about to be released, I'm also actively looking at ways to optimize FRED entry. One way is to precompute certain useful values in the entry path, instead of simply zeroing the GPRs one can extract data that will be needed further on in the data path into the argument registers.

While true, this works until we need our first mitigation in the FRED
entrypath, and then everything goes to pot.

>
> Note that FRED entry is an architectural speculation barrier.

What are the semantics of this new type of barrier?

We were previously given guarantees that ring changes didn't execute
speculatively, which is good enough for every concern we could think of
at the time.

FRED's behaviour of not even reloading %cs/%ss means that in principle
it could speculate straight through a decode-time #UD/#DB/#CP/etc at
full pelt.  I'm glad to hear that it doesn't.

> Furthermore, my measurements so far indicate that doing an early-out on the SYSCALL path is pretty essential. It might even be valuable enough to do in the assembly stub code. 

I was wondering about how the nested switch statements would fair.

But, given how much nicer everything else is writing it in C, what about
a decent run with PGO, or some manual __attribute__((hot)) for SYSCALL
(from Userspace) and #PF (from supervisor) to help the compiler out with
the fastpaths?

~Andrew

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.