[PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive

Sohil Mehta posted 15 patches 4 months ago
There is a newer version of this series
[PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive
Posted by Sohil Mehta 4 months ago
From: Yian Chen <yian.chen@intel.com>

Security features such as LASS are not expected to be disabled once
initialized. Add LASS to the CR4 pinned mask.

Signed-off-by: Yian Chen <yian.chen@intel.com>
Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>
---
v10:
 - No change.
---
 arch/x86/kernel/cpu/common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index c7d3512914ca..61ab332eaf73 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -403,7 +403,8 @@ static __always_inline void setup_umip(struct cpuinfo_x86 *c)
 
 /* These bits should not change their value after CPU init is finished. */
 static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_UMIP |
-					     X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED;
+					     X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED |
+					     X86_CR4_LASS;
 static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning);
 static unsigned long cr4_pinned_bits __ro_after_init;
 
-- 
2.43.0
Re: [PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive
Posted by Edgecombe, Rick P 4 months ago
On Mon, 2025-10-06 at 23:51 -0700, Sohil Mehta wrote:
> From: Yian Chen <yian.chen@intel.com>
> 
> Security features such as LASS are not expected to be disabled once
> initialized. Add LASS to the CR4 pinned mask.
> 
> Signed-off-by: Yian Chen <yian.chen@intel.com>
> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com>

I was debating whether we really need this, given the LASS and CR pinning threat
models. CR pinning seems to be about after an attacker has already hijacked a
control flow and is looking to escalate it into more control. We could maybe get
away with dropping this and the following patch. But it would still be good to
get a warning if it gets turned off inadvertently I think. It might be worth
adding justification like that to the log.
Re: [PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive
Posted by Sohil Mehta 4 months ago
On 10/7/2025 11:24 AM, Edgecombe, Rick P wrote:
>> Security features such as LASS are not expected to be disabled once
>> initialized. Add LASS to the CR4 pinned mask.
>>
> 
> I was debating whether we really need this, given the LASS and CR pinning threat
> models. CR pinning seems to be about after an attacker has already hijacked a
> control flow and is looking to escalate it into more control.

Can you please explain more? How is LASS different from SMAP and SMEP
for which the CR pinning code was initially added?

> We could maybe get
> away with dropping this and the following patch. But it would still be good to
> get a warning if it gets turned off inadvertently I think. It might be worth
> adding justification like that to the log.

My understanding from the previous discussions was that CR pinning
deferral might be beneficial independent of LASS.
https://lore.kernel.org/lkml/c59aa7ac-62a6-45ec-b626-de518b25f7d9@intel.com/

The pinning enforcement provides the warning and reprograms the bit.
Maybe, I've misunderstood your comment.
Re: [PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive
Posted by Edgecombe, Rick P 4 months ago
On Tue, 2025-10-07 at 16:11 -0700, Sohil Mehta wrote:
> On 10/7/2025 11:24 AM, Edgecombe, Rick P wrote:
> > > Security features such as LASS are not expected to be disabled once
> > > initialized. Add LASS to the CR4 pinned mask.
> > > 
> > 
> > I was debating whether we really need this, given the LASS and CR pinning threat
> > models. CR pinning seems to be about after an attacker has already hijacked a
> > control flow and is looking to escalate it into more control.
> 
> Can you please explain more? How is LASS different from SMAP and SMEP
> for which the CR pinning code was initially added?

The next patch says "CR pinning mainly prevents exploits from trivially
modifying security-sensitive CR bits."

My understanding of that attack is that attacker already has enough control in
the kernel to call CR4 writing functions, or otherwise control the CR4 write.
They disable SMAP or something to help do ROP for the next step of their attack.

I *think* the observation that lead to CR pinning was that before SMAP attacks
would prep a ROP chain (stack) in userspace memory, then use a function pointer
highjack to call a function that switched the stack to userspace memory. After
SMAP this was blocked, and attacks had to do the longer step of forming and
finding the ROP stack in kernel memory. But then some observed attacks were just
first calling CR4 writing functions to disable SMAP and then continue the
userspace based attack like normal. So CR pinning could block this and force
them the other way. Then as long as we had the infrastructure, any CR bits that
might help were added to the mask because why not. (I think this is the history,
but please don't take it as authoritative)

Over SMAP, LASS has speculative benefits. Usually a speculative attack doesn't
involve non-speculative control flow highjack. If you already have that, you
probably don't need to mess with a speculative attack. (hand waiving a bit)

So I was thinking that the CR pinning of LASS doesn't really help that reasoning
from the next patch. And unlike the other bits that just got added easily, this
one required infrastructure changes and extra patch. So wondered, hmm, is it
worth it to do the extra patches?

> 
> > We could maybe get
> > away with dropping this and the following patch. But it would still be good to
> > get a warning if it gets turned off inadvertently I think. It might be worth
> > adding justification like that to the log.
> 
> My understanding from the previous discussions was that CR pinning
> deferral might be beneficial independent of LASS.
> https://lore.kernel.org/lkml/c59aa7ac-62a6-45ec-b626-de518b25f7d9@intel.com/
> 
> The pinning enforcement provides the warning and reprograms the bit.
> Maybe, I've misunderstood your comment.
> 

Yea, I agree it would be good to get a warning. The write may be triggered
accidentally by a kernel bug. I agree with the patch, but just commenting my
reasoning for the sake of discussion. Maybe we can tighten the reasoning in the
log. I tend to think that if I have to go into a long chain of analysis to
decide I agree with the patch, that the log should have helped me get there. Of
course this can also just be because it went over my head. Please take it as a
soft comment.
Re: [PATCH v10 04/15] x86/cpu: Set LASS CR4 bit as pinning sensitive
Posted by Sohil Mehta 4 months ago
On 10/8/2025 9:52 AM, Edgecombe, Rick P wrote:
> And unlike the other bits that just got added easily, this
> one required infrastructure changes and extra patch. So wondered, hmm, is it
> worth it to do the extra patches?
> 

Yeah, I reconsidered its usefulness as well.

Though, adding it to CR4 pinned mask is a net positive, even with the
extra patch. As part of the pinning enforcement, we get the warning if
LASS is turned off by accident. The bit gets reprogrammed automatically
which, even if not useful, wouldn't harm in any way.

Also, the changes to defer CR pinning enforcement seem to be useful
independent of LASS. Plus, the patch turned out to be simpler than I had
imagined.

> Yea, I agree it would be good to get a warning. The write may be triggered
> accidentally by a kernel bug. I agree with the patch, but just commenting my
> reasoning for the sake of discussion. Maybe we can tighten the reasoning in the
> log.
Will do.