Re: [PATCHv6 08/16] x86/traps: Consolidate user fixups in exc_general_protection()

Re: [PATCHv6 08/16] x86/traps: Consolidate user fixups in exc_general_protection()

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> @@ -819,6 +814,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>  		if (fixup_vdso_exception(regs, X86_TRAP_GP, error_code, 0))
>  			goto exit;
>  
> +		if (cpu_feature_enabled(X86_FEATURE_UMIP) && fixup_umip_exception(regs))
> +			goto exit;

I'd move the cpu_feature_enabled() check into fixup_umip_exception()
just to make the exc_general_protection() code easier to look at. It'll
force an extra (cheap) CALL, but this is a super cold code path anyway.

With that:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

Re: [PATCHv6 13/16] x86/traps: Handle LASS thrown #SS

[Xin Li]

On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
> From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> 
> LASS throws a #GP for any violations except for stack register accesses,
> in which case it throws a #SS instead. Handle this similarly to how other
> LASS violations are handled.
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>   arch/x86/kernel/traps.c | 34 ++++++++++++++++++++++++++++------
>   1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 0f558d3369a3..bd8f7e72b238 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -866,6 +860,34 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
>   	cond_local_irq_disable(regs);
>   }
>   
> +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> +{
> +	if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> +		enum kernel_gp_hint hint = GP_NO_HINT;
> +		unsigned long gp_addr;
> +
> +		if (user_mode(regs)) {
> +			gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, GPFSTR);
> +			return;
> +		}

Please add the following FRED code:

		if (cpu_feature_enabled(X86_FEATURE_FRED) &&
		    fixup_exception(regs, X86_TRAP_SS, error_code, 0))
			return;

Because the following FRED fixup

  
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5105e7687ad3dffde77f6e4393b5530e83d672dc

is otherwise skipped, it is interpreted as a LASS #SS before reaching
the FRED fixup (which is indeed a userspace fault but triggered from
a specific kernel IP as described in the commit changelog).

The x86 selftest sigreturn_64 fails w/o the above fix.

> +
> +		hint = get_kernel_gp_address(regs, &gp_addr);
> +		if (hint != GP_NO_HINT) {
> +			printk(GPFSTR ", %s 0x%lx", kernel_gp_hint_help[hint],

GPFSTR?

#define GPFSTR "general protection fault"

It is misleading here, you need something like SSFSTR which tells
exactly which exception caused it.

Thanks!
     Xin

Re: [PATCHv6 13/16] x86/traps: Handle LASS thrown #SS

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:20:12AM -0700, Xin Li wrote:
> On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
> > From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > 
> > LASS throws a #GP for any violations except for stack register accesses,
> > in which case it throws a #SS instead. Handle this similarly to how other
> > LASS violations are handled.
> > 
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > ---
> >   arch/x86/kernel/traps.c | 34 ++++++++++++++++++++++++++++------
> >   1 file changed, 28 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> > index 0f558d3369a3..bd8f7e72b238 100644
> > --- a/arch/x86/kernel/traps.c
> > +++ b/arch/x86/kernel/traps.c
> > @@ -866,6 +860,34 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
> >   	cond_local_irq_disable(regs);
> >   }
> > +DEFINE_IDTENTRY_ERRORCODE(exc_stack_segment)
> > +{
> > +	if (cpu_feature_enabled(X86_FEATURE_LASS)) {
> > +		enum kernel_gp_hint hint = GP_NO_HINT;
> > +		unsigned long gp_addr;
> > +
> > +		if (user_mode(regs)) {
> > +			gp_user_force_sig_segv(regs, X86_TRAP_GP, error_code, GPFSTR);
> > +			return;
> > +		}
> 
> Please add the following FRED code:
> 
> 		if (cpu_feature_enabled(X86_FEATURE_FRED) &&
> 		    fixup_exception(regs, X86_TRAP_SS, error_code, 0))
> 			return;
> 
> Because the following FRED fixup
> 
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5105e7687ad3dffde77f6e4393b5530e83d672dc
> 
> is otherwise skipped, it is interpreted as a LASS #SS before reaching
> the FRED fixup (which is indeed a userspace fault but triggered from
> a specific kernel IP as described in the commit changelog).
> 
> The x86 selftest sigreturn_64 fails w/o the above fix.

Okay, will do.

> > +
> > +		hint = get_kernel_gp_address(regs, &gp_addr);
> > +		if (hint != GP_NO_HINT) {
> > +			printk(GPFSTR ", %s 0x%lx", kernel_gp_hint_help[hint],
> 
> GPFSTR?
> 
> #define GPFSTR "general protection fault"
> 
> It is misleading here, you need something like SSFSTR which tells
> exactly which exception caused it.

Yep, thanks for catching that!

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 03/16] x86/alternatives: Disable LASS when patching kernel alternatives

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> +/*
> + * poking_init() initializes the text poking address from the lower half of the
> + * address space. Relax LASS enforcement when accessing the poking address.
> + */

This does not read quite right. I think there's some unnecessary
confusiuon about initializing the address versus the contents. But
either way, how about:

/*
 * Text poking creates and uses a mapping in the lower half of the
 * address space. Relax LASS enforcement when accessing the poking
 * address.
 */

>  static void text_poke_memset(void *dst, const void *src, size_t len)
>  {
>  	int c = *(const int *)src;
>  
> -	memset(dst, c, len);
> +	lass_stac();
> +	__inline_memset(dst, c, len);
> +	lass_clac();
>  }

Yeah, this patch small enough that lass_{stac,clac}() are definitely
misplaced in this series. Just move them into this patch.

Re: [PATCHv6 03/16] x86/alternatives: Disable LASS when patching kernel alternatives

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:33:57AM -0700, Dave Hansen wrote:
> On 6/20/25 06:53, Kirill A. Shutemov wrote:
> > +/*
> > + * poking_init() initializes the text poking address from the lower half of the
> > + * address space. Relax LASS enforcement when accessing the poking address.
> > + */
> 
> This does not read quite right. I think there's some unnecessary
> confusiuon about initializing the address versus the contents. But
> either way, how about:
> 
> /*
>  * Text poking creates and uses a mapping in the lower half of the
>  * address space. Relax LASS enforcement when accessing the poking
>  * address.
>  */

Looks good. Will update, thanks!
> 
> >  static void text_poke_memset(void *dst, const void *src, size_t len)
> >  {
> >  	int c = *(const int *)src;
> >  
> > -	memset(dst, c, len);
> > +	lass_stac();
> > +	__inline_memset(dst, c, len);
> > +	lass_clac();
> >  }
> 
> Yeah, this patch small enough that lass_{stac,clac}() are definitely
> misplaced in this series. Just move them into this patch.

Okay, makes sense.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 04/16] x86/efi: Move runtime service initialization to arch/x86

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> 
> The EFI call in start_kernel() is guarded by #ifdef CONFIG_X86. Move
> the thing to the arch_cpu_finalize_init() path on x86 and get rid of
> the #ifdef in start_kernel().

What does this have to do with LASS?

Is there any reason not to just pull this in to tip now as a cleanup? It
looks good regardless of what it does to LASS.

Re: [PATCHv6 04/16] x86/efi: Move runtime service initialization to arch/x86

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:35:14AM -0700, Dave Hansen wrote:
> On 6/20/25 06:53, Kirill A. Shutemov wrote:
> > From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > 
> > The EFI call in start_kernel() is guarded by #ifdef CONFIG_X86. Move
> > the thing to the arch_cpu_finalize_init() path on x86 and get rid of
> > the #ifdef in start_kernel().
> 
> What does this have to do with LASS?

It is required to move CR pinning below it and allow to disable CR4.LASS
temporarily during EFI initialization.

> Is there any reason not to just pull this in to tip now as a cleanup? It
> looks good regardless of what it does to LASS.

I am totally fine with applying it right away.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> +static __always_inline void lass_clac(void)
> +{
> +	alternative("", "clac", X86_FEATURE_LASS);
> +}
> +
> +static __always_inline void lass_stac(void)
> +{
> +	alternative("", "stac", X86_FEATURE_LASS);
> +}

There are differing opinions on how to introduce code. Do you do it up
front and then use it later in the series? Or do you only introduce
infrastructure when you use it?

Doing what you have here (introduce first, use later) is fine. But it
does incur an increased burden of explaining things.

These two functions in particular mean that you have to at least allude
to how these get used later in the series.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
> index 250c10627ab3..9574c198fc08 100644
> --- a/arch/x86/Kconfig.cpufeatures
> +++ b/arch/x86/Kconfig.cpufeatures
> @@ -124,6 +124,10 @@ config X86_DISABLED_FEATURE_PCID
>   	def_bool y
>   	depends on !X86_64
>   
> +config X86_DISABLED_FEATURE_LASS
> +	def_bool y
> +	depends on !X86_64
> +
>   config X86_DISABLED_FEATURE_PKU
>   	def_bool y
>   	depends on !X86_INTEL_MEMORY_PROTECTION_KEYS

You don't need to add X86_DISABLED_FEATURE_LASS, because the LASS code
is NOT optional at build time, i.e., you now don't have CONFIG_X86_LASS.

It was removed per the change log:

Changes from v1[1]:
- Remove CONFIG_X86_LASS

Thanks!
     Xin

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:36:30AM -0700, Xin Li wrote:
> On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
> > index 250c10627ab3..9574c198fc08 100644
> > --- a/arch/x86/Kconfig.cpufeatures
> > +++ b/arch/x86/Kconfig.cpufeatures
> > @@ -124,6 +124,10 @@ config X86_DISABLED_FEATURE_PCID
> >   	def_bool y
> >   	depends on !X86_64
> > +config X86_DISABLED_FEATURE_LASS
> > +	def_bool y
> > +	depends on !X86_64
> > +
> >   config X86_DISABLED_FEATURE_PKU
> >   	def_bool y
> >   	depends on !X86_INTEL_MEMORY_PROTECTION_KEYS
> 
> You don't need to add X86_DISABLED_FEATURE_LASS, because the LASS code
> is NOT optional at build time, i.e., you now don't have CONFIG_X86_LASS.

Hmm. But it is optional. It depends on CONFIG_X86_64. I don't think we
want it to be advertised on 32-bit kernels.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/20/2025 10:31 AM, Kirill A. Shutemov wrote:
> On Fri, Jun 20, 2025 at 08:36:30AM -0700, Xin Li wrote:
>> On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
>>> diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
>>> index 250c10627ab3..9574c198fc08 100644
>>> --- a/arch/x86/Kconfig.cpufeatures
>>> +++ b/arch/x86/Kconfig.cpufeatures
>>> @@ -124,6 +124,10 @@ config X86_DISABLED_FEATURE_PCID
>>>    	def_bool y
>>>    	depends on !X86_64
>>> +config X86_DISABLED_FEATURE_LASS
>>> +	def_bool y
>>> +	depends on !X86_64
>>> +
>>>    config X86_DISABLED_FEATURE_PKU
>>>    	def_bool y
>>>    	depends on !X86_INTEL_MEMORY_PROTECTION_KEYS
>>
>> You don't need to add X86_DISABLED_FEATURE_LASS, because the LASS code
>> is NOT optional at build time, i.e., you now don't have CONFIG_X86_LASS.
> 
> Hmm. But it is optional. It depends on CONFIG_X86_64. I don't think we
> want it to be advertised on 32-bit kernels.
> 

I kind of ignore 32-bit...

But I simply hate adding a disabled feature that depends on !X86_64;
x86_64 has a broad scope, and new CPU features are often intentionally
not enabled for 32-bit.

(X86_DISABLED_FEATURE_PCID is the only one before LASS)

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On June 20, 2025 4:46:21 PM PDT, Xin Li <xin@zytor.com> wrote:
>On 6/20/2025 10:31 AM, Kirill A. Shutemov wrote:
>> On Fri, Jun 20, 2025 at 08:36:30AM -0700, Xin Li wrote:
>>> On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
>>>> diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
>>>> index 250c10627ab3..9574c198fc08 100644
>>>> --- a/arch/x86/Kconfig.cpufeatures
>>>> +++ b/arch/x86/Kconfig.cpufeatures
>>>> @@ -124,6 +124,10 @@ config X86_DISABLED_FEATURE_PCID
>>>>    	def_bool y
>>>>    	depends on !X86_64
>>>> +config X86_DISABLED_FEATURE_LASS
>>>> +	def_bool y
>>>> +	depends on !X86_64
>>>> +
>>>>    config X86_DISABLED_FEATURE_PKU
>>>>    	def_bool y
>>>>    	depends on !X86_INTEL_MEMORY_PROTECTION_KEYS
>>> 
>>> You don't need to add X86_DISABLED_FEATURE_LASS, because the LASS code
>>> is NOT optional at build time, i.e., you now don't have CONFIG_X86_LASS.
>> 
>> Hmm. But it is optional. It depends on CONFIG_X86_64. I don't think we
>> want it to be advertised on 32-bit kernels.
>> 
>
>I kind of ignore 32-bit...
>
>But I simply hate adding a disabled feature that depends on !X86_64;
>x86_64 has a broad scope, and new CPU features are often intentionally
>not enabled for 32-bit.
>
>(X86_DISABLED_FEATURE_PCID is the only one before LASS)
>
>

More importantly, it is wrong. 

The 32-bit build can depend on this feature not existing, therefore it SHOULD be listed as a disabled feature.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On 2025-06-20 17:45, H. Peter Anvin wrote:
>>
>> But I simply hate adding a disabled feature that depends on !X86_64;
>> x86_64 has a broad scope, and new CPU features are often intentionally
>> not enabled for 32-bit.
>>
>> (X86_DISABLED_FEATURE_PCID is the only one before LASS)
> 
> More importantly, it is wrong.
> 
> The 32-bit build can depend on this feature not existing, therefore it SHOULD be listed as a disabled feature.
> 

Ok, that was word salad. What I meant was that the original patch is 
correct, and we SHOULD have this as a disabled feature.

The reason is that it reduces the need to explicitly test for 32/64 bits 
for features that don't exist on 32 bits. When they are flagged as 
disabled, they get filtered out *at compile time*.

	-hpa

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/20/2025 5:50 PM, H. Peter Anvin wrote:
> On 2025-06-20 17:45, H. Peter Anvin wrote:
>>>
>>> But I simply hate adding a disabled feature that depends on !X86_64;
>>> x86_64 has a broad scope, and new CPU features are often intentionally
>>> not enabled for 32-bit.
>>>
>>> (X86_DISABLED_FEATURE_PCID is the only one before LASS)
>>
>> More importantly, it is wrong.
>>
>> The 32-bit build can depend on this feature not existing, therefore it 
>> SHOULD be listed as a disabled feature.
>>
> 
> Ok, that was word salad. What I meant was that the original patch is 
> correct, and we SHOULD have this as a disabled feature.

Agreed!

> The reason is that it reduces the need to explicitly test for 32/64 bits 
> for features that don't exist on 32 bits. When they are flagged as 
> disabled, they get filtered out *at compile time*.

It's better to make it depend on X86_32 directly rather than !X86_64:

config X86_DISABLED_FEATURE_LASS
	def_bool y
	depends on X86_32


But the disabled feature list due to lack of 32-bit enabling will keep
growing until we remove 32-bit kernel code.

Wondering should we bother enforcing cpuid_deps[] on 32-bit?

IOW, turn off the feature when its dependency isn’t satisfied on 32b-it;
don’t just throw a warning and hope for the best.

Thanks!
     Xin

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On June 23, 2025 10:40:59 AM PDT, Xin Li <xin@zytor.com> wrote:
>On 6/20/2025 5:50 PM, H. Peter Anvin wrote:
>> On 2025-06-20 17:45, H. Peter Anvin wrote:
>>>> 
>>>> But I simply hate adding a disabled feature that depends on !X86_64;
>>>> x86_64 has a broad scope, and new CPU features are often intentionally
>>>> not enabled for 32-bit.
>>>> 
>>>> (X86_DISABLED_FEATURE_PCID is the only one before LASS)
>>> 
>>> More importantly, it is wrong.
>>> 
>>> The 32-bit build can depend on this feature not existing, therefore it SHOULD be listed as a disabled feature.
>>> 
>> 
>> Ok, that was word salad. What I meant was that the original patch is correct, and we SHOULD have this as a disabled feature.
>
>Agreed!
>
>> The reason is that it reduces the need to explicitly test for 32/64 bits for features that don't exist on 32 bits. When they are flagged as disabled, they get filtered out *at compile time*.
>
>It's better to make it depend on X86_32 directly rather than !X86_64:
>
>config X86_DISABLED_FEATURE_LASS
>	def_bool y
>	depends on X86_32
>
>
>But the disabled feature list due to lack of 32-bit enabling will keep
>growing until we remove 32-bit kernel code.
>
>Wondering should we bother enforcing cpuid_deps[] on 32-bit?
>
>IOW, turn off the feature when its dependency isn’t satisfied on 32b-it;
>don’t just throw a warning and hope for the best.
>
>Thanks!
>    Xin
>

We should have the dependencies enforced; in fact, preferably we would enforce them at build time as well.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/23/2025 7:04 PM, H. Peter Anvin wrote:
> On June 23, 2025 10:40:59 AM PDT, Xin Li <xin@zytor.com> wrote:
>> On 6/20/2025 5:50 PM, H. Peter Anvin wrote:
>>> On 2025-06-20 17:45, H. Peter Anvin wrote:
>>>>>
>>>>> But I simply hate adding a disabled feature that depends on !X86_64;
>>>>> x86_64 has a broad scope, and new CPU features are often intentionally
>>>>> not enabled for 32-bit.
>>>>>
>>>>> (X86_DISABLED_FEATURE_PCID is the only one before LASS)
>>>>
>>>> More importantly, it is wrong.
>>>>
>>>> The 32-bit build can depend on this feature not existing, therefore it SHOULD be listed as a disabled feature.
>>>>
>>>
>>> Ok, that was word salad. What I meant was that the original patch is correct, and we SHOULD have this as a disabled feature.
>>
>> Agreed!
>>
>>> The reason is that it reduces the need to explicitly test for 32/64 bits for features that don't exist on 32 bits. When they are flagged as disabled, they get filtered out *at compile time*.
>>
>> It's better to make it depend on X86_32 directly rather than !X86_64:
>>
>> config X86_DISABLED_FEATURE_LASS
>> 	def_bool y
>> 	depends on X86_32
>>
>>
>> But the disabled feature list due to lack of 32-bit enabling will keep
>> growing until we remove 32-bit kernel code.
>>
>> Wondering should we bother enforcing cpuid_deps[] on 32-bit?
>>
>> IOW, turn off the feature when its dependency isn’t satisfied on 32b-it;
>> don’t just throw a warning and hope for the best.
>>
>> Thanks!
>>     Xin
>>
> 
> We should have the dependencies enforced; in fact, preferably we would enforce them at build time as well.
> 
> 

Yeah, sounds something we can do later :)

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/23/2025 9:57 PM, Xin Li wrote:
> On 6/23/2025 7:04 PM, H. Peter Anvin wrote:
>> On June 23, 2025 10:40:59 AM PDT, Xin Li <xin@zytor.com> wrote:
>>> On 6/20/2025 5:50 PM, H. Peter Anvin wrote:
>>>> On 2025-06-20 17:45, H. Peter Anvin wrote:
>>>>>>
>>>>>> But I simply hate adding a disabled feature that depends on !X86_64;
>>>>>> x86_64 has a broad scope, and new CPU features are often 
>>>>>> intentionally
>>>>>> not enabled for 32-bit.
>>>>>>
>>>>>> (X86_DISABLED_FEATURE_PCID is the only one before LASS)
>>>>>
>>>>> More importantly, it is wrong.
>>>>>
>>>>> The 32-bit build can depend on this feature not existing, therefore 
>>>>> it SHOULD be listed as a disabled feature.
>>>>>
>>>>
>>>> Ok, that was word salad. What I meant was that the original patch is 
>>>> correct, and we SHOULD have this as a disabled feature.
>>>
>>> Agreed!
>>>
>>>> The reason is that it reduces the need to explicitly test for 32/64 
>>>> bits for features that don't exist on 32 bits. When they are flagged 
>>>> as disabled, they get filtered out *at compile time*.
>>>
>>> It's better to make it depend on X86_32 directly rather than !X86_64:
>>>
>>> config X86_DISABLED_FEATURE_LASS
>>>     def_bool y
>>>     depends on X86_32
>>>
>>>
>>> But the disabled feature list due to lack of 32-bit enabling will keep
>>> growing until we remove 32-bit kernel code.
>>>
>>> Wondering should we bother enforcing cpuid_deps[] on 32-bit?
>>>
>>> IOW, turn off the feature when its dependency isn’t satisfied on 32b-it;
>>> don’t just throw a warning and hope for the best.
>>>
>>> Thanks!
>>>     Xin
>>>
>>
>> We should have the dependencies enforced; in fact, preferably we would 
>> enforce them at build time as well.
>>
>>
> 
> Yeah, sounds something we can do later :)
> 

We could introduce a new Kconfig file at

     arch/x86/Kconfig.cpufeatures.disabled_on_32bit

to track all features disabled due to lack of 32-bit support.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Randy Dunlap]

Hi--

On 6/20/25 6:53 AM, Kirill A. Shutemov wrote:
> From: Sohil Mehta <sohil.mehta@intel.com>
> 
> Linear Address Space Separation (LASS) is a security feature that
> intends to prevent malicious virtual address space accesses across
> user/kernel mode.
> 
> Such mode based access protection already exists today with paging and
> features such as SMEP and SMAP. However, to enforce these protections,
> the processor must traverse the paging structures in memory.  Malicious
> software can use timing information resulting from this traversal to
> determine details about the paging structures, and these details may
> also be used to determine the layout of the kernel memory.
> 
> The LASS mechanism provides the same mode-based protections as paging
> but without traversing the paging structures. Because the protections
> enforced by LASS are applied before paging, software will not be able to
> derive paging-based timing information from the various caching
> structures such as the TLBs, mid-level caches, page walker, data caches,
> etc.
> 
> LASS enforcement relies on the typical kernel implementation to divide
> the 64-bit virtual address space into two halves:
>   Addr[63]=0 -> User address space
>   Addr[63]=1 -> Kernel address space
> 
> Any data access or code execution across address spaces typically
> results in a #GP fault.
> 
> The LASS enforcement for kernel data access is dependent on CR4.SMAP
> being set. The enforcement can be disabled by toggling the RFLAGS.AC bit
> similar to SMAP.
> 
> Define the CPU feature bits to enumerate this feature and include
> feature dependencies to reflect the same.
> 
> Co-developed-by: Yian Chen <yian.chen@intel.com>
> Signed-off-by: Yian Chen <yian.chen@intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> ---
>  arch/x86/Kconfig.cpufeatures                |  4 ++++
>  arch/x86/include/asm/cpufeatures.h          |  1 +
>  arch/x86/include/asm/smap.h                 | 22 +++++++++++++++++++--
>  arch/x86/include/uapi/asm/processor-flags.h |  2 ++
>  arch/x86/kernel/cpu/cpuid-deps.c            |  1 +
>  tools/arch/x86/include/asm/cpufeatures.h    |  1 +
>  6 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/Kconfig.cpufeatures b/arch/x86/Kconfig.cpufeatures
> index 250c10627ab3..9574c198fc08 100644
> --- a/arch/x86/Kconfig.cpufeatures
> +++ b/arch/x86/Kconfig.cpufeatures
> @@ -124,6 +124,10 @@ config X86_DISABLED_FEATURE_PCID
>  	def_bool y
>  	depends on !X86_64
>  
> +config X86_DISABLED_FEATURE_LASS
> +	def_bool y
> +	depends on !X86_64

Please explain why this is   !X86_64.
Thanks.

> +
>  config X86_DISABLED_FEATURE_PKU
>  	def_bool y
>  	depends on !X86_INTEL_MEMORY_PROTECTION_KEYS


-- 
~Randy

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Xin Li]

On 6/20/2025 9:02 AM, Randy Dunlap wrote:
>> +config X86_DISABLED_FEATURE_LASS
>> +	def_bool y
>> +	depends on !X86_64
> Please explain why this is   !X86_64.

When NOT on X86_64, the LASS code should not be compiled.

But first of all, as I replied earlier, X86_DISABLED_FEATURE_LASS is
completely not needed.

Thanks!
     Xin

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Randy Dunlap]

On 6/20/25 9:12 AM, Xin Li wrote:
> On 6/20/2025 9:02 AM, Randy Dunlap wrote:
>>> +config X86_DISABLED_FEATURE_LASS
>>> +    def_bool y
>>> +    depends on !X86_64
>> Please explain why this is   !X86_64.
> 
> When NOT on X86_64, the LASS code should not be compiled.

Ah yes, the double negative caught me.

> But first of all, as I replied earlier, X86_DISABLED_FEATURE_LASS is
> completely not needed.

That's good.

Thanks.

-- 
~Randy

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Fri, Jun 20, 2025 at 04:53:09PM +0300, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index ee176236c2be..4473a6f7800b 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -313,6 +313,7 @@
>  #define X86_FEATURE_SM4			(12*32+ 2) /* SM4 instructions */
>  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* "avx_vnni" AVX VNNI instructions */
>  #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* "avx512_bf16" AVX512 BFLOAT16 instructions */
> +#define X86_FEATURE_LASS		(12*32+ 6) /* "lass" Linear Address Space Separation */

This needs to be in /proc/cpuinfo because...?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 06:35:04PM +0200, Borislav Petkov wrote:
> On Fri, Jun 20, 2025 at 04:53:09PM +0300, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > index ee176236c2be..4473a6f7800b 100644
> > --- a/arch/x86/include/asm/cpufeatures.h
> > +++ b/arch/x86/include/asm/cpufeatures.h
> > @@ -313,6 +313,7 @@
> >  #define X86_FEATURE_SM4			(12*32+ 2) /* SM4 instructions */
> >  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* "avx_vnni" AVX VNNI instructions */
> >  #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* "avx512_bf16" AVX512 BFLOAT16 instructions */
> > +#define X86_FEATURE_LASS		(12*32+ 6) /* "lass" Linear Address Space Separation */
> 
> This needs to be in /proc/cpuinfo because...?

What is current policy around it ?

I think it is useful to advertise security features in cpuinfo. LASS fits
next to SMAP/SMEP/UMIP.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Fri, Jun 20, 2025 at 08:33:36PM +0300, Kirill A. Shutemov wrote:
> What is current policy around it ?

Documentation/arch/x86/cpuinfo.rst

> I think it is useful to advertise security features in cpuinfo.

Because who's going to consume them?

Don't get me wrong - I am trying to see whether the whole use case for this is
well thought out. Because it becomes an ABI.

But if no one is going to use it, why bother?

Arguably, for this thing the argument would be - as it is put in that file
above:

"So, the current use of /proc/cpuinfo is to show features which the
kernel has *enabled* and *supports*."

as it has been enabled by machinery.

So that's ok. I'm just making sure we're on the same page and you're not
aiming at something completely different with this.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:29:43PM +0200, Borislav Petkov wrote:
> On Fri, Jun 20, 2025 at 08:33:36PM +0300, Kirill A. Shutemov wrote:
> > What is current policy around it ?
> 
> Documentation/arch/x86/cpuinfo.rst
> 
> > I think it is useful to advertise security features in cpuinfo.
> 
> Because who's going to consume them?
> 
> Don't get me wrong - I am trying to see whether the whole use case for this is
> well thought out. Because it becomes an ABI.
> 
> But if no one is going to use it, why bother?
> 
> Arguably, for this thing the argument would be - as it is put in that file
> above:
> 
> "So, the current use of /proc/cpuinfo is to show features which the
> kernel has *enabled* and *supports*."
> 
> as it has been enabled by machinery.
> 
> So that's ok. I'm just making sure we're on the same page and you're not
> aiming at something completely different with this.

What about this:

LASS provides protection against a class of speculative attacks, such as
SLAM[1]. Add the "lass" flag to /proc/cpuinfo to indicate that the feature
is supported by hardware and enabled by the kernel. This allows userspace
to determine if the setup is secure against such attacks.

[1] https://download.vusec.net/papers/slam_sp24.pdf

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Mon, Jun 23, 2025 at 11:17:02AM +0300, Kirill A. Shutemov wrote:
> What about this:
> 
> LASS provides protection against a class of speculative attacks, such as
> SLAM[1]. Add the "lass" flag to /proc/cpuinfo to indicate that the feature
> is supported by hardware and enabled by the kernel. This allows userspace
> to determine if the setup is secure against such attacks.

Yeah, thanks.

I'm still not fully on board with userspace determining whether they're
mitigated or not but that's a general problem with our mitigations.

Also, I haven't looked at the patchset yet but I think it should be also
adding code to bugs.c to make all those vulns which it addresses, report that
they're mitigated by LASS now in

grep -r . /sys/devices/system/cpu/vulnerabilities/

output.

Which makes your cpuinfo flag not really needed as we already have a special
method for the mitigations reporting.

But ok, it has gotten kernel enablement so stating so in cpuinfo is ok.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Kirill A. Shutemov]

On Mon, Jun 23, 2025 at 12:21:05PM +0200, Borislav Petkov wrote:
> On Mon, Jun 23, 2025 at 11:17:02AM +0300, Kirill A. Shutemov wrote:
> > What about this:
> > 
> > LASS provides protection against a class of speculative attacks, such as
> > SLAM[1]. Add the "lass" flag to /proc/cpuinfo to indicate that the feature
> > is supported by hardware and enabled by the kernel. This allows userspace
> > to determine if the setup is secure against such attacks.
> 
> Yeah, thanks.
> 
> I'm still not fully on board with userspace determining whether they're
> mitigated or not but that's a general problem with our mitigations.
> 
> Also, I haven't looked at the patchset yet but I think it should be also
> adding code to bugs.c to make all those vulns which it addresses, report that
> they're mitigated by LASS now in
> 
> grep -r . /sys/devices/system/cpu/vulnerabilities/
> 
> output.
> 
> Which makes your cpuinfo flag not really needed as we already have a special
> method for the mitigations reporting.
> 
> But ok, it has gotten kernel enablement so stating so in cpuinfo is ok.

Due to SLAM, we decided to postpone LAM enabling, until LASS is landed.

I am not sure if we want to add static
/sys/devices/system/cpu/vulnerabilities/slam with "Mitigation: LASS".

There might be other yet-to-be-discovered speculative attacks that LASS
mitigates. Security features have to visible to userspace independently of
known vulnerabilities.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Mon, Jun 23, 2025 at 04:42:41PM +0300, Kirill A. Shutemov wrote:
> Due to SLAM, we decided to postpone LAM enabling, until LASS is landed.
> 
> I am not sure if we want to add static
> /sys/devices/system/cpu/vulnerabilities/slam with "Mitigation: LASS".
> 
> There might be other yet-to-be-discovered speculative attacks that LASS
> mitigates. Security features have to visible to userspace independently of
> known vulnerabilities.

... and the fact that a vuln is being mitigated by stating that in
/sys/devices/system/cpu/vulnerabilities/ needs to happen too.

I'm not talking about LAM enablement - I'm talking about adding a

SPECTRE_V1_MITIGATION_LASS

and setting that when X86_FEATURE_LASS is set so that luserspace gets told
that

"Spectre V1 : Mitigation: LASS"

or so.

Makes more sense?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Thu, Jun 26, 2025 at 05:18:37PM +0200, Borislav Petkov wrote:
> On Mon, Jun 23, 2025 at 04:42:41PM +0300, Kirill A. Shutemov wrote:
> > Due to SLAM, we decided to postpone LAM enabling, until LASS is landed.
> > 
> > I am not sure if we want to add static
> > /sys/devices/system/cpu/vulnerabilities/slam with "Mitigation: LASS".
> > 
> > There might be other yet-to-be-discovered speculative attacks that LASS
> > mitigates. Security features have to visible to userspace independently of
> > known vulnerabilities.
> 
> ... and the fact that a vuln is being mitigated by stating that in
> /sys/devices/system/cpu/vulnerabilities/ needs to happen too.
> 
> I'm not talking about LAM enablement - I'm talking about adding a
> 
> SPECTRE_V1_MITIGATION_LASS
> 
> and setting that when X86_FEATURE_LASS is set so that luserspace gets told
> that
> 
> "Spectre V1 : Mitigation: LASS"
> 
> or so.
> 
> Makes more sense?

I meant this crap, ofc:

        switch (bug) {
        case X86_BUG_CPU_MELTDOWN:
                if (boot_cpu_has(X86_FEATURE_PTI))
                        return sysfs_emit(buf, "Mitigation: PTI\n");

This should say "Mitigation: LASS" if LASS is enabled...

Which begs the question: how do LASS and PTI interact now?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Dave Hansen]

On 6/26/25 09:07, Borislav Petkov wrote:
>> Makes more sense?
> I meant this crap, ofc:
> 
>         switch (bug) {
>         case X86_BUG_CPU_MELTDOWN:
>                 if (boot_cpu_has(X86_FEATURE_PTI))
>                         return sysfs_emit(buf, "Mitigation: PTI\n");
> 
> This should say "Mitigation: LASS" if LASS is enabled...
> 
> Which begs the question: how do LASS and PTI interact now?

Maybe my babbling about LASS mitigation Meltdown was ill considered. It
seems that I've just muddied the waters.

All the real LASS-capable hardware also has RDCL_NO=1 which is the
_actual_ x86 Meltdown mitigation. Those systems are not vulnerable to
Meltdown in the first place.

They should say: "Not affected" for Meltdown, both before and after LASS.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Kirill A. Shutemov]

On Thu, Jun 26, 2025 at 10:21:23AM -0700, Dave Hansen wrote:
> On 6/26/25 09:07, Borislav Petkov wrote:
> >> Makes more sense?
> > I meant this crap, ofc:
> > 
> >         switch (bug) {
> >         case X86_BUG_CPU_MELTDOWN:
> >                 if (boot_cpu_has(X86_FEATURE_PTI))
> >                         return sysfs_emit(buf, "Mitigation: PTI\n");
> > 
> > This should say "Mitigation: LASS" if LASS is enabled...
> > 
> > Which begs the question: how do LASS and PTI interact now?
> 
> Maybe my babbling about LASS mitigation Meltdown was ill considered. It
> seems that I've just muddied the waters.
> 
> All the real LASS-capable hardware also has RDCL_NO=1 which is the
> _actual_ x86 Meltdown mitigation. Those systems are not vulnerable to
> Meltdown in the first place.
> 
> They should say: "Not affected" for Meltdown, both before and after LASS.

Right. To best of my knowledge, SLAM is the only known vulnerability LASS
fixes directly so far.

So, we want an entry for SLAM?

I don't think it is very useful as we don't allow LAM if LASS is missing.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Borislav Petkov]

On Fri, Jun 27, 2025 at 01:25:12PM +0300, Kirill A. Shutemov wrote:
> So, we want an entry for SLAM?
> 
> I don't think it is very useful as we don't allow LAM if LASS is missing.

Nah, it is all clear now.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Dave Hansen]

On 6/27/25 03:25, Kirill A. Shutemov wrote:
> So, we want an entry for SLAM?

SLAM wasn't ever a practical problem anywhere, so I don't think we need
to do anything on top of what we already did.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Sohil Mehta]

On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
>  
> +/*
> + * The CLAC/STAC instructions toggle enforcement of X86_FEATURE_SMAP.
> + *
> + * X86_FEATURE_LASS requires flipping the AC flag when accessing the lower half
> + * of the virtual address space, regardless of the _PAGE_BIT_USER bit in the
> + * page tables. lass_clac/stac() should be used for these cases.
> + *

Is this supposed to be "regardless" or only when the _PAGE_BIT_USER bit
it set? The way the sentence is worded it would seem that the kernel
could always use lass_clac()/stac() since the value in _PAGE_BIT_USER
doesn't matter.

Please correct me if I am wrong, but here is my understanding:

X86_FEATURE_SMAP and X86_FEATURE_LASS both complain when the kernel
tries to access the lower half of the virtual addresses.

SMAP flags an issue if _PAGE_BIT_USER is not set. LASS would #GP in both
cases with or without the _PAGE_BIT_USER being set.

However, in terms of usage, we want to use LASS specific stac()/clac()
only when _PAGE_BIT_USER is set. Since this won't be flagged by SMAP.

@Dave Hansen, you had suggested separating out the SMAP/LASS AC toggle
functions. But, the difference in usage between both of them seems very
subtle. Could this be easily misused?

For example, there is no failure that would happen if someone
incorrectly uses the SMAP specific clac()/stac() calls instead of the
LASS ones.

> + * Note: a barrier is implicit in alternative().
> + */
> +
>  static __always_inline void clac(void)
>  {
> -	/* Note: a barrier is implicit in alternative() */
>  	alternative("", "clac", X86_FEATURE_SMAP);
>  }
>  
>  static __always_inline void stac(void)
>  {
> -	/* Note: a barrier is implicit in alternative() */
>  	alternative("", "stac", X86_FEATURE_SMAP);
>  }
>  
> +static __always_inline void lass_clac(void)
> +{
> +	alternative("", "clac", X86_FEATURE_LASS);
> +}
> +
> +static __always_inline void lass_stac(void)
> +{
> +	alternative("", "stac", X86_FEATURE_LASS);
> +}
> +

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Dave Hansen]

On 6/20/25 11:14, Sohil Mehta wrote:
> @Dave Hansen, you had suggested separating out the SMAP/LASS AC toggle
> functions. But, the difference in usage between both of them seems very
> subtle. Could this be easily misused?

Logically there are two completely different things:

	1. Touching userspace
	2. Touching the lower half of the address space

If it's only userspace in the lower half of the address space, then
there's no controversy. But the problem obviously occurs when you want
to touch kernel mappings in the lower half of the address space.

I want to preserve the "stac/clas" meaning as just "touch userspace".

The new functions should be for "touch the lower half of the address
space" alone. Maybe it should be:

	lass_disable_enforcement()
	lass_enable_enforcement()

The only downside of not having stac/clac in the names is that it's not
obvious that they have an impact on SMAP because they're named to be
LASS-only. But I'm not super worried about this. If we have a
proliferation of call sites we have bigger problem on our hands.

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Sohil Mehta]

On 6/20/2025 11:24 AM, Dave Hansen wrote:
> On 6/20/25 11:14, Sohil Mehta wrote:
>> @Dave Hansen, you had suggested separating out the SMAP/LASS AC toggle
>> functions. But, the difference in usage between both of them seems very
>> subtle. Could this be easily misused?
> 
> Logically there are two completely different things:
> 
> 	1. Touching userspace
> 	2. Touching the lower half of the address space
> 
> If it's only userspace in the lower half of the address space, then
> there's no controversy. But the problem obviously occurs when you want
> to touch kernel mappings in the lower half of the address space.
> 

Makes sense. If we decide to go this way, we should capture some of this
thinking into the kernel comments. I find the current wording very
ambiguous.

> I want to preserve the "stac/clas" meaning as just "touch userspace".
> 
> The new functions should be for "touch the lower half of the address
> space" alone. Maybe it should be:
> 
> 	lass_disable_enforcement()
> 	lass_enable_enforcement()
> 
> The only downside of not having stac/clac in the names is that it's not
> obvious that they have an impact on SMAP because they're named to be
> LASS-only. 

Yeah, the reverse is also true, where the SMAP specific clac()/stac()
toggle LASS enforcement :)

RE: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Luck, Tony]

> > functions. But, the difference in usage between both of them seems very
> > subtle. Could this be easily misused?
>
> Logically there are two completely different things:
>
>       1. Touching userspace
>       2. Touching the lower half of the address space
>
> If it's only userspace in the lower half of the address space, then
> there's no controversy. But the problem obviously occurs when you want
> to touch kernel mappings in the lower half of the address space.

Why does the kernel create the mappings to poke kernel text
for ALTERNATIVE patching in the lower half of the address space?

Instead of special "we really to want to access the lower addresses"
code, wouldn't it be easier to map the "poke" virtual addresses in normal
kernel upper-half space?

-Tony

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Dave Hansen]

On 6/23/25 09:25, Luck, Tony wrote:
>>> functions. But, the difference in usage between both of them seems very
>>> subtle. Could this be easily misused?
>>
>> Logically there are two completely different things:
>>
>>       1. Touching userspace
>>       2. Touching the lower half of the address space
>>
>> If it's only userspace in the lower half of the address space, then
>> there's no controversy. But the problem obviously occurs when you want
>> to touch kernel mappings in the lower half of the address space.
> 
> Why does the kernel create the mappings to poke kernel text
> for ALTERNATIVE patching in the lower half of the address space?
> 
> Instead of special "we really to want to access the lower addresses"
> code, wouldn't it be easier to map the "poke" virtual addresses in normal
> kernel upper-half space?

The upper half of the address space is shared kernel space, right? Every
PGD has identical contents in the upper half. So if we create a mapping
there,everybody get access to it. Every mm can access it. Every
*process* can access it. It still has kernel permissions of course, but
it's still a place that everybody can get at.

The lower half is *ONLY* accessible to the local mm. In this case, only
the text poking mm. It's a natural, safe, place to create a mapping that
you want to be private and not be exploited.

So, doing it in the upper half is risky.

If we *wanted*, we could have a non-shared PGD entry in the top half of
the address space. But we'd need to reserve its address space and all
that jazz. I'm not sure it's any better than just disabling LASS
enforcement for a moment.

RE: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Luck, Tony]

> >> Logically there are two completely different things:
> >>
> >>       1. Touching userspace
> >>       2. Touching the lower half of the address space
> >>
> >> If it's only userspace in the lower half of the address space, then
> >> there's no controversy. But the problem obviously occurs when you want
> >> to touch kernel mappings in the lower half of the address space.
> >
> > Why does the kernel create the mappings to poke kernel text
> > for ALTERNATIVE patching in the lower half of the address space?
> >
> > Instead of special "we really to want to access the lower addresses"
> > code, wouldn't it be easier to map the "poke" virtual addresses in normal
> > kernel upper-half space?
>
> The upper half of the address space is shared kernel space, right? Every
> PGD has identical contents in the upper half. So if we create a mapping
> there,everybody get access to it. Every mm can access it. Every
> *process* can access it. It still has kernel permissions of course, but
> it's still a place that everybody can get at.
>
> The lower half is *ONLY* accessible to the local mm. In this case, only
> the text poking mm. It's a natural, safe, place to create a mapping that
> you want to be private and not be exploited.
>
> So, doing it in the upper half is risky.
>
> If we *wanted*, we could have a non-shared PGD entry in the top half of
> the address space. But we'd need to reserve its address space and all
> that jazz. I'm not sure it's any better than just disabling LASS
> enforcement for a moment.

Maybe it’s a thing to put on the list for "when x86 drops support for 32-bit".

Reserving a PGD entry in the kernel half of the address space for
local CPU use would be practical then. Perhaps there might be other
uses too.

-Tony

RE: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On June 23, 2025 4:13:34 PM PDT, "Luck, Tony" <tony.luck@intel.com> wrote:
>> >> Logically there are two completely different things:
>> >>
>> >>       1. Touching userspace
>> >>       2. Touching the lower half of the address space
>> >>
>> >> If it's only userspace in the lower half of the address space, then
>> >> there's no controversy. But the problem obviously occurs when you want
>> >> to touch kernel mappings in the lower half of the address space.
>> >
>> > Why does the kernel create the mappings to poke kernel text
>> > for ALTERNATIVE patching in the lower half of the address space?
>> >
>> > Instead of special "we really to want to access the lower addresses"
>> > code, wouldn't it be easier to map the "poke" virtual addresses in normal
>> > kernel upper-half space?
>>
>> The upper half of the address space is shared kernel space, right? Every
>> PGD has identical contents in the upper half. So if we create a mapping
>> there,everybody get access to it. Every mm can access it. Every
>> *process* can access it. It still has kernel permissions of course, but
>> it's still a place that everybody can get at.
>>
>> The lower half is *ONLY* accessible to the local mm. In this case, only
>> the text poking mm. It's a natural, safe, place to create a mapping that
>> you want to be private and not be exploited.
>>
>> So, doing it in the upper half is risky.
>>
>> If we *wanted*, we could have a non-shared PGD entry in the top half of
>> the address space. But we'd need to reserve its address space and all
>> that jazz. I'm not sure it's any better than just disabling LASS
>> enforcement for a moment.
>
>Maybe it’s a thing to put on the list for "when x86 drops support for 32-bit".
>
>Reserving a PGD entry in the kernel half of the address space for
>local CPU use would be practical then. Perhaps there might be other
>uses too.
>
>-Tony
>

Are we actually doing patching on more than one CPU at a time?

RE: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[Luck, Tony]

> Are we actually doing patching on more than one CPU at a time?

We could call static_key_enable()/static_key_disable() for different
keys at the same time from multiple CPUs.

Do we actually do that? Probably not.

-Tony

RE: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On June 23, 2025 5:10:50 PM PDT, "Luck, Tony" <tony.luck@intel.com> wrote:
>> Are we actually doing patching on more than one CPU at a time?
>
>We could call static_key_enable()/static_key_disable() for different
>keys at the same time from multiple CPUs.
>
>Do we actually do that? Probably not.
>
>-Tony
>

But does it actually *work*?

Re: [PATCHv6 01/16] x86/cpu: Enumerate the LASS feature bits

[H. Peter Anvin]

On June 20, 2025 11:14:56 AM PDT, Sohil Mehta <sohil.mehta@intel.com> wrote:
>On 6/20/2025 6:53 AM, Kirill A. Shutemov wrote:
>>  
>> +/*
>> + * The CLAC/STAC instructions toggle enforcement of X86_FEATURE_SMAP.
>> + *
>> + * X86_FEATURE_LASS requires flipping the AC flag when accessing the lower half
>> + * of the virtual address space, regardless of the _PAGE_BIT_USER bit in the
>> + * page tables. lass_clac/stac() should be used for these cases.
>> + *
>
>Is this supposed to be "regardless" or only when the _PAGE_BIT_USER bit
>it set? The way the sentence is worded it would seem that the kernel
>could always use lass_clac()/stac() since the value in _PAGE_BIT_USER
>doesn't matter.
>
>Please correct me if I am wrong, but here is my understanding:
>
>X86_FEATURE_SMAP and X86_FEATURE_LASS both complain when the kernel
>tries to access the lower half of the virtual addresses.
>
>SMAP flags an issue if _PAGE_BIT_USER is not set. LASS would #GP in both
>cases with or without the _PAGE_BIT_USER being set.
>
>However, in terms of usage, we want to use LASS specific stac()/clac()
>only when _PAGE_BIT_USER is set. Since this won't be flagged by SMAP.
>
>@Dave Hansen, you had suggested separating out the SMAP/LASS AC toggle
>functions. But, the difference in usage between both of them seems very
>subtle. Could this be easily misused?
>
>For example, there is no failure that would happen if someone
>incorrectly uses the SMAP specific clac()/stac() calls instead of the
>LASS ones.
>
>> + * Note: a barrier is implicit in alternative().
>> + */
>> +
>>  static __always_inline void clac(void)
>>  {
>> -	/* Note: a barrier is implicit in alternative() */
>>  	alternative("", "clac", X86_FEATURE_SMAP);
>>  }
>>  
>>  static __always_inline void stac(void)
>>  {
>> -	/* Note: a barrier is implicit in alternative() */
>>  	alternative("", "stac", X86_FEATURE_SMAP);
>>  }
>>  
>> +static __always_inline void lass_clac(void)
>> +{
>> +	alternative("", "clac", X86_FEATURE_LASS);
>> +}
>> +
>> +static __always_inline void lass_stac(void)
>> +{
>> +	alternative("", "stac", X86_FEATURE_LASS);
>> +}
>> +

"Regardless" is correct. LASS only considers which hemisphere the virtual address is located in, because it is explicitly designed to prevent walking the page tables in the "wrong" hemisphere and therefore speculative accesses that happen to form pointers into user space addresses will not cause TLB or cache fills that might be possible to probe.

The obvious exception is when the kernel is intentionally performing accesses on behalf of user space, which is exactly what SMAP tells the hardware already.

Re: [PATCHv6 00/16] x86: Enable Linear Address Space Separation support

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> Linear Address Space Separation (LASS) is a security feature that intends to
> prevent malicious virtual address space accesses across user/kernel mode.

Most of the cover letter here reads like an Intel whitepaper. That's not
the worst thing in the world, but I think it glosses over one very
important point:

	Had it been available, LASS alone would have mitigated Meltdown.

Could we say this up front in a prominent place, please?

Re: [PATCHv6 00/16] x86: Enable Linear Address Space Separation support

[Andrew Cooper]

> Most of the cover letter here reads like an Intel whitepaper. That's not
> the worst thing in the world, but I think it glosses over one very
> important point:
>
> 	Had it been available, LASS alone would have mitigated Meltdown.
>
> Could we say this up front in a prominent place, please?

I'm going to nitpick. :)

Yes, LASS would have made Meltdown a far less major problem than it was,
but I don't think that phrasing is fair.  As I recall, LASS was
literally invented as a "what would have been useful?" exercise in the
wake of Meltdown.

However, a less well known/researched area of Meltdown, which would not
be addressed by LASS, is the ability to pend stores to read-only memory
and proceed with the expectation that they'll success.

Other things which would have helped would have been PKS (and this *was*
asked for ahead of Skylake launching...)

The other important thing about LASS is that it does cut off a whole
class of sidechannels.  This halts definitely-rogue speculation, but is
useful for non-speculative security too.

~Andrew

Re: [PATCHv6 00/16] x86: Enable Linear Address Space Separation support

[H. Peter Anvin]

On June 20, 2025 3:04:53 PM PDT, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> Most of the cover letter here reads like an Intel whitepaper. That's not
>> the worst thing in the world, but I think it glosses over one very
>> important point:
>>
>> 	Had it been available, LASS alone would have mitigated Meltdown.
>>
>> Could we say this up front in a prominent place, please?
>
>I'm going to nitpick. :)
>
>Yes, LASS would have made Meltdown a far less major problem than it was,
>but I don't think that phrasing is fair.  As I recall, LASS was
>literally invented as a "what would have been useful?" exercise in the
>wake of Meltdown.
>
>However, a less well known/researched area of Meltdown, which would not
>be addressed by LASS, is the ability to pend stores to read-only memory
>and proceed with the expectation that they'll success.
>
>Other things which would have helped would have been PKS (and this *was*
>asked for ahead of Skylake launching...)
>
>The other important thing about LASS is that it does cut off a whole
>class of sidechannels.  This halts definitely-rogue speculation, but is
>useful for non-speculative security too.
>
>~Andrew

Could you clarify what you mean with "pend stores to read-only memory?"

Re: [PATCHv6 00/16] x86: Enable Linear Address Space Separation support

[H. Peter Anvin]

On June 20, 2025 3:04:53 PM PDT, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> Most of the cover letter here reads like an Intel whitepaper. That's not
>> the worst thing in the world, but I think it glosses over one very
>> important point:
>>
>> 	Had it been available, LASS alone would have mitigated Meltdown.
>>
>> Could we say this up front in a prominent place, please?
>
>I'm going to nitpick. :)
>
>Yes, LASS would have made Meltdown a far less major problem than it was,
>but I don't think that phrasing is fair.  As I recall, LASS was
>literally invented as a "what would have been useful?" exercise in the
>wake of Meltdown.
>
>However, a less well known/researched area of Meltdown, which would not
>be addressed by LASS, is the ability to pend stores to read-only memory
>and proceed with the expectation that they'll success.
>
>Other things which would have helped would have been PKS (and this *was*
>asked for ahead of Skylake launching...)
>
>The other important thing about LASS is that it does cut off a whole
>class of sidechannels.  This halts definitely-rogue speculation, but is
>useful for non-speculative security too.
>
>~Andrew

To some degree, the best way to describe LASS is "KPTI in hardware." No, it isn't an exact analogy – we don't switch the entire page table structure – but it does make the supervisor page table structure protected from user space *while still in memory*.

It is coarser-grained than software KPTI, but on the other hand the separation is absolute; there is to keep things like the kernel stack, descriptor tables and entry/exit code unprotected.

Re: [PATCHv6 00/16] x86: Enable Linear Address Space Separation support

[Dave Hansen]

On 6/20/25 15:04, Andrew Cooper wrote:
>> Most of the cover letter here reads like an Intel whitepaper. That's not
>> the worst thing in the world, but I think it glosses over one very
>> important point:
>>
>> 	Had it been available, LASS alone would have mitigated Meltdown.
>>
>> Could we say this up front in a prominent place, please?
> 
> I'm going to nitpick. :)
> 
> Yes, LASS would have made Meltdown a far less major problem than it was,
> but I don't think that phrasing is fair.  As I recall, LASS was
> literally invented as a "what would have been useful?" exercise in the
> wake of Meltdown.

While being concise, I was also trying to convey that LASS is pretty
powerful, rather than it being some uniquely genius move by the CPU
designers. It was absolutely conceived of in hindsight.

Do you feel like the changelog is sufficient as-is? Or do you think we
can beef the changelog up a bit? I just want to make sure folks reading
it have a clear understanding that it's not fixing purely theoretical
issues.

Re: [PATCHv6 05/16] x86/cpu: Defer CR pinning setup until after EFI initialization

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
>  	/*
>  	 * This needs to follow the FPU initializtion, since EFI depends on it.
> +	 *
> +	 * It also needs to precede the CR pinning setup, because the CR4.LASS
> +	 * bit has to be cleared temporarily in order to execute the
> +	 * set_virtual_address_map() EFI call, which resides in lower addresses
> +	 * and would trip LASS if enabled.
> +	 *
> +	 * Wrapping efi_enter_virtual_mode() into lass_stac()/clac() is not
> +	 * enough because AC flag gates data accesses, but not instruction
> +	 * fetch. Clearing the CR4 bit is required.
>  	 */
>  	if (efi_enabled(EFI_RUNTIME_SERVICES))
>  		efi_enter_virtual_mode();
>  
> +	setup_cr_pinning();

This is a _bit_ too much of a comment for me in this context. This would
be enough:

	/* EFI twiddles CR4.LASS. Do it before CR pinning: */
  	if (efi_enabled(EFI_RUNTIME_SERVICES))
  		efi_enter_virtual_mode();

	setup_cr_pinning();

I also dislike comments where code will do. Could
efi_enter_virtual_mode() actually check for CR4.LASS? Or check
cr4_pinned_bits() or 'cr_pinning.key'?

Also, the "lass_stac()/clac() is not enough" comment seems more like
changelog material to me than something for a comment.

Re: [PATCHv6 06/16] efi: Disable LASS around set_virtual_address_map() EFI call

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 463b784499a8..94c335229697 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -825,11 +825,24 @@ static void __init __efi_enter_virtual_mode(void)
>  
>  	efi_sync_low_kernel_mappings();
>  
> +	/*
> +	 * set_virtual_address_map is the only service located at lower
> +	 * addresses, so we have to temporarily disable LASS around it.
> +	 * Note that clearing EFLAGS.AC is not enough for this, the whole
> +	 * LASS needs to be disabled.
> +	 */
> +	if (cpu_feature_enabled(X86_FEATURE_LASS))
> +		cr4_clear_bits(X86_CR4_LASS);

Could we do it like this instead?

	unsigned long lass = cr4_read_shadow() & X86_FEATURE_LASS;
	...
	cr4_clear_bits(lass);

>  	status = efi_set_virtual_address_map(efi.memmap.desc_size * count,
>  					     efi.memmap.desc_size,
>  					     efi.memmap.desc_version,
>  					     (efi_memory_desc_t *)pa,
>  					     efi_systab_phys);
> +
> +	if (cpu_feature_enabled(X86_FEATURE_LASS))
> +		cr4_set_bits(X86_CR4_LASS);

and:

	cr4_set_bits(lass);

>  	if (status != EFI_SUCCESS) {
>  		pr_err("Unable to switch EFI into virtual mode (status=%lx)!\n",
>  		       status);

That way, neither the presence of X86_FEATURE_LASS nor the ordering of
setting up X86_CR4_LASS matters.

Let's say the CPU supports X86_FEATURE_LASS and this code gets called
before the kernel is ready for LASS. It would break as written above.

Re: [PATCHv6 06/16] efi: Disable LASS around set_virtual_address_map() EFI call

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 08:55:29AM -0700, Dave Hansen wrote:
> On 6/20/25 06:53, Kirill A. Shutemov wrote:
> > diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> > index 463b784499a8..94c335229697 100644
> > --- a/arch/x86/platform/efi/efi.c
> > +++ b/arch/x86/platform/efi/efi.c
> > @@ -825,11 +825,24 @@ static void __init __efi_enter_virtual_mode(void)
> >  
> >  	efi_sync_low_kernel_mappings();
> >  
> > +	/*
> > +	 * set_virtual_address_map is the only service located at lower
> > +	 * addresses, so we have to temporarily disable LASS around it.
> > +	 * Note that clearing EFLAGS.AC is not enough for this, the whole
> > +	 * LASS needs to be disabled.
> > +	 */
> > +	if (cpu_feature_enabled(X86_FEATURE_LASS))
> > +		cr4_clear_bits(X86_CR4_LASS);
> 
> Could we do it like this instead?
> 
> 	unsigned long lass = cr4_read_shadow() & X86_FEATURE_LASS;
> 	...
> 	cr4_clear_bits(lass);
> 
> >  	status = efi_set_virtual_address_map(efi.memmap.desc_size * count,
> >  					     efi.memmap.desc_size,
> >  					     efi.memmap.desc_version,
> >  					     (efi_memory_desc_t *)pa,
> >  					     efi_systab_phys);
> > +
> > +	if (cpu_feature_enabled(X86_FEATURE_LASS))
> > +		cr4_set_bits(X86_CR4_LASS);
> 
> and:
> 
> 	cr4_set_bits(lass);
> 
> >  	if (status != EFI_SUCCESS) {
> >  		pr_err("Unable to switch EFI into virtual mode (status=%lx)!\n",
> >  		       status);
> 
> That way, neither the presence of X86_FEATURE_LASS nor the ordering of
> setting up X86_CR4_LASS matters.
> 
> Let's say the CPU supports X86_FEATURE_LASS and this code gets called
> before the kernel is ready for LASS. It would break as written above.

Okay, will rework this.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Dave Hansen]

On 6/20/25 06:53, Kirill A. Shutemov wrote:
> +bool emulate_vsyscall_pf(unsigned long error_code, struct pt_regs *regs,
> +			 unsigned long address)
> +{
> +	/* Write faults or kernel-privilege faults never get fixed up. */
> +	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
> +		return false;
> +
> +	if (!(error_code & X86_PF_INSTR)) {
> +		/* Failed vsyscall read */
> +		if (vsyscall_mode == EMULATE)
> +			return false;
> +
> +		/*
> +		 * User code tried and failed to read the vsyscall page.
> +		 */
> +		warn_bad_vsyscall(KERN_INFO, regs,
> +				  "vsyscall read attempt denied -- look up the vsyscall kernel parameter if you need a workaround");
> +		return false;
> +	}
> +
> +	return __emulate_vsyscall(regs, address);
> +}

For this patch that just moves the code:

Acked-by: Dave Hansen <dave.hansen@linux.intel.com>

But, the resulting code is wonky. It needs to do something more like this:

	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
		return false;

	if (error_code & X86_PF_INSTR))
		return __emulate_vsyscall(regs, address);

	/* Failed vsyscall read */
	if (vsyscall_mode == EMULATE)
		return false;

	/*
	 * User code tried and failed to read the vsyscall page.
	 */
	warn_bad_vsyscall(KERN_INFO, regs, ...
	return false;

That's much more linear to read.

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Andrew Cooper]

> But, the resulting code is wonky. It needs to do something more like this:
>
> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
> 		return false;
>
> 	if (error_code & X86_PF_INSTR))
> 		return __emulate_vsyscall(regs, address);

To do this, LASS needs a proper interlink against NX || SMEP.

If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
meaning that fetches are reported as plain reads.

This leads to some fun corner cases in SMAP and now LASS too for virt.

~Andrew

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Sohil Mehta]

On 6/20/2025 4:08 PM, Andrew Cooper wrote:
>> But, the resulting code is wonky. It needs to do something more like this:
>>
>> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
>> 		return false;
>>
>> 	if (error_code & X86_PF_INSTR))
>> 		return __emulate_vsyscall(regs, address);
> 
> To do this, LASS needs a proper interlink against NX || SMEP.
> 
> If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
> meaning that fetches are reported as plain reads.
> 
> This leads to some fun corner cases in SMAP and now LASS too for virt.


Maybe I am missing something, but LASS works pre-paging so it wouldn't
generate a PF, right?

We have a new vsyscall emulation for #GP (patch 9) that wouldn't follow
this path.

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Andrew Cooper]

On 21/06/2025 12:18 am, Sohil Mehta wrote:
> On 6/20/2025 4:08 PM, Andrew Cooper wrote:
>>> But, the resulting code is wonky. It needs to do something more like this:
>>>
>>> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
>>> 		return false;
>>>
>>> 	if (error_code & X86_PF_INSTR))
>>> 		return __emulate_vsyscall(regs, address);
>> To do this, LASS needs a proper interlink against NX || SMEP.
>>
>> If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
>> meaning that fetches are reported as plain reads.
>>
>> This leads to some fun corner cases in SMAP and now LASS too for virt.
> Maybe I am missing something, but LASS works pre-paging so it wouldn't
> generate a PF, right?

Oh right, yes.  This is a preexisting bug in vsyscall #PF handling.

It simply became obvious with Dave's suggested rearrangement.

~Andrew

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Dave Hansen]

On 6/20/25 16:08, Andrew Cooper wrote:
>> But, the resulting code is wonky. It needs to do something more like this:
>>
>> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
>> 		return false;
>>
>> 	if (error_code & X86_PF_INSTR))
>> 		return __emulate_vsyscall(regs, address);
> To do this, LASS needs a proper interlink against NX || SMEP.
> 
> If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
> meaning that fetches are reported as plain reads.
Interesting point.

I think the easiest way to do this is just make a cpuid_deps[] entry for
LASS and NX. If there's a CPU where LASS is available but where NX isn't
available, we have much bigger problems on our hands.

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[H. Peter Anvin]

On June 20, 2025 4:21:38 PM PDT, Dave Hansen <dave.hansen@intel.com> wrote:
>On 6/20/25 16:08, Andrew Cooper wrote:
>>> But, the resulting code is wonky. It needs to do something more like this:
>>>
>>> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
>>> 		return false;
>>>
>>> 	if (error_code & X86_PF_INSTR))
>>> 		return __emulate_vsyscall(regs, address);
>> To do this, LASS needs a proper interlink against NX || SMEP.
>> 
>> If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
>> meaning that fetches are reported as plain reads.
>Interesting point.
>
>I think the easiest way to do this is just make a cpuid_deps[] entry for
>LASS and NX. If there's a CPU where LASS is available but where NX isn't
>available, we have much bigger problems on our hands.

Indeed. There is exactly zero reason to support this case.

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Kirill A. Shutemov]

On Fri, Jun 20, 2025 at 04:21:38PM -0700, Dave Hansen wrote:
> On 6/20/25 16:08, Andrew Cooper wrote:
> >> But, the resulting code is wonky. It needs to do something more like this:
> >>
> >> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
> >> 		return false;
> >>
> >> 	if (error_code & X86_PF_INSTR))
> >> 		return __emulate_vsyscall(regs, address);
> > To do this, LASS needs a proper interlink against NX || SMEP.
> > 
> > If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
> > meaning that fetches are reported as plain reads.
> Interesting point.
> 
> I think the easiest way to do this is just make a cpuid_deps[] entry for
> LASS and NX. If there's a CPU where LASS is available but where NX isn't
> available, we have much bigger problems on our hands.

I am not sure what I suppose to do here.

Sohil pointed out that with LASS we get #GP on vsyscall, not #PF and PFEC
is not relevant for LASS.

So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
vsyscall on boot if NX is not available?

BTW, why do we even support !NX on X86_64? Is there such HW?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Andrew Cooper]

On 23/06/2025 1:41 pm, Kirill A. Shutemov wrote:
> On Fri, Jun 20, 2025 at 04:21:38PM -0700, Dave Hansen wrote:
>> On 6/20/25 16:08, Andrew Cooper wrote:
>>>> But, the resulting code is wonky. It needs to do something more like this:
>>>>
>>>> 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
>>>> 		return false;
>>>>
>>>> 	if (error_code & X86_PF_INSTR))
>>>> 		return __emulate_vsyscall(regs, address);
>>> To do this, LASS needs a proper interlink against NX || SMEP.
>>>
>>> If neither NX nor SMEP are active, the CPU does not report X86_PF_INSTR,
>>> meaning that fetches are reported as plain reads.
>> Interesting point.
>>
>> I think the easiest way to do this is just make a cpuid_deps[] entry for
>> LASS and NX. If there's a CPU where LASS is available but where NX isn't
>> available, we have much bigger problems on our hands.
> I am not sure what I suppose to do here.
>
> Sohil pointed out that with LASS we get #GP on vsyscall, not #PF and PFEC
> is not relevant for LASS.

Correct.  That was my mistake originally.

>
> So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
> vsyscall on boot if NX is not available?
>
> BTW, why do we even support !NX on X86_64? Is there such HW?

Yes.  Early P4 steppings had no NX at all.

~Andrew

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Dave Hansen]

[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]

On 6/23/25 05:41, Kirill A. Shutemov wrote:
> So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
> vsyscall on boot if NX is not available?

Well, vsyscall=none can break old userspace, so forcing it on old
hardware doesn't seem like a great idea.

But, either way, this doesn't really appear to be a LASS issue. This code:

>         if (!(error_code & X86_PF_INSTR)) {
>                 /* Failed vsyscall read */
>                 if (vsyscall_mode == EMULATE)
>                         return false;

Is really asking the question:

	Is this #PF from an instruction fetch in the vsyscall page?

That _should_ be able to be done by comparing CR2 and regs->rip. In
fact, that's done just below anyway:

	WARN_ON_ONCE(address != regs->ip);

So I think we can fix this up with something like the attached patch
which just drives the if() from regs->rip and make the warning NX-only.

But this code has been like this a long time and I'm 99% sure the x86
selftests poke at all these cases. I'm curious what they do on those old
P4's (or a 64-bit VM with NX turned off), but it's not super important
either way.

[-- Attachment #2: vsyscall-nx.patch --]
[-- Type: text/x-patch, Size: 1152 bytes --]

diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
index c9103a6fa06e..0b0e0283994f 100644
--- a/arch/x86/entry/vsyscall/vsyscall_64.c
+++ b/arch/x86/entry/vsyscall/vsyscall_64.c
@@ -124,7 +124,8 @@ bool emulate_vsyscall(unsigned long error_code,
 	if ((error_code & (X86_PF_WRITE | X86_PF_USER)) != X86_PF_USER)
 		return false;
 
-	if (!(error_code & X86_PF_INSTR)) {
+	/* Avoid emulation unless userspace was executing from vsyscall page: */
+	if (address != regs->ip) {
 		/* Failed vsyscall read */
 		if (vsyscall_mode == EMULATE)
 			return false;
@@ -136,13 +137,16 @@ bool emulate_vsyscall(unsigned long error_code,
 		return false;
 	}
 
+
+	/* X86_PF_INSTR is only set when NX is supported: */
+	if (cpu_feature_enabled(X86_FEATURE_NX))
+		WARN_ON_ONCE(!(error_code & X86_PF_INSTR));
+
 	/*
 	 * No point in checking CS -- the only way to get here is a user mode
 	 * trap to a high address, which means that we're in 64-bit user code.
 	 */
 
-	WARN_ON_ONCE(address != regs->ip);
-
 	if (vsyscall_mode == NONE) {
 		warn_bad_vsyscall(KERN_INFO, regs,
 				  "vsyscall attempted with vsyscall=none");

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Andrew Cooper]

On 23/06/2025 4:32 pm, Dave Hansen wrote:
> On 6/23/25 05:41, Kirill A. Shutemov wrote:
>> So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
>> vsyscall on boot if NX is not available?
> Well, vsyscall=none can break old userspace, so forcing it on old
> hardware doesn't seem like a great idea.
>
> But, either way, this doesn't really appear to be a LASS issue. This code:
>
>>         if (!(error_code & X86_PF_INSTR)) {
>>                 /* Failed vsyscall read */
>>                 if (vsyscall_mode == EMULATE)
>>                         return false;
> Is really asking the question:
>
> 	Is this #PF from an instruction fetch in the vsyscall page?
>
> That _should_ be able to be done by comparing CR2 and regs->rip. In
> fact, that's done just below anyway:
>
> 	WARN_ON_ONCE(address != regs->ip);
>
> So I think we can fix this up with something like the attached patch
> which just drives the if() from regs->rip and make the warning NX-only.

Yeah, that looks good.  Furthermore, it means that the LASS #GP path
(patch 9) will be consistent with this path.  (i.e. both doing a
regs->rip check.)

Patch Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> if that
counts for anything.

~Andrew

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Kirill A. Shutemov]

On Mon, Jun 23, 2025 at 08:32:53AM -0700, Dave Hansen wrote:
> On 6/23/25 05:41, Kirill A. Shutemov wrote:
> > So, IIUC, that's dependency of vsyscall PF on NX. Do we want to disable
> > vsyscall on boot if NX is not available?
> 
> Well, vsyscall=none can break old userspace, so forcing it on old
> hardware doesn't seem like a great idea.
> 
> But, either way, this doesn't really appear to be a LASS issue. This code:
> 
> >         if (!(error_code & X86_PF_INSTR)) {
> >                 /* Failed vsyscall read */
> >                 if (vsyscall_mode == EMULATE)
> >                         return false;
> 
> Is really asking the question:
> 
> 	Is this #PF from an instruction fetch in the vsyscall page?
> 
> That _should_ be able to be done by comparing CR2 and regs->rip. In
> fact, that's done just below anyway:
> 
> 	WARN_ON_ONCE(address != regs->ip);
> 
> So I think we can fix this up with something like the attached patch
> which just drives the if() from regs->rip and make the warning NX-only.

Looks good to me.

Do you want me to include it into this patchset or will you apply it
separately?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCHv6 07/16] x86/vsyscall: Reorganize the #PF emulation code

[Dave Hansen]

On 6/24/25 04:37, Kirill A. Shutemov wrote:
> Do you want me to include it into this patchset or will you apply it
> separately?

Actually, if you want to break it out and just submit it separately,
I'll probably just apply it.