Skip to content

Commit ae6012d

Browse files
arunarhansendc
authored andcommitted
x86/pkeys: Ensure updated PKRU value is XRSTOR'd
When XSTATE_BV[i] is 0, and XRSTOR attempts to restore state component 'i' it ignores any value in the XSAVE buffer and instead restores the state component's init value. This means that if XSAVE writes XSTATE_BV[PKRU]=0 then XRSTOR will ignore the value that update_pkru_in_sigframe() writes to the XSAVE buffer. XSTATE_BV[PKRU] only gets written as 0 if PKRU is in its init state. On Intel CPUs, basically never happens because the kernel usually overwrites the init value (aside: this is why we didn't notice this bug until now). But on AMD, the init tracker is more aggressive and will track PKRU as being in its init state upon any wrpkru(0x0). Unfortunately, sig_prepare_pkru() does just that: wrpkru(0x0). This writes XSTATE_BV[PKRU]=0 which makes XRSTOR ignore the PKRU value in the sigframe. To fix this, always overwrite the sigframe XSTATE_BV with a value that has XSTATE_BV[PKRU]==1. This ensures that XRSTOR will not ignore what update_pkru_in_sigframe() wrote. The problematic sequence of events is something like this: Userspace does: * wrpkru(0xffff0000) (or whatever) * Hardware sets: XINUSE[PKRU]=1 Signal happens, kernel is entered: * sig_prepare_pkru() => wrpkru(0x00000000) * Hardware sets: XINUSE[PKRU]=0 (aggressive AMD init tracker) * XSAVE writes most of XSAVE buffer, including XSTATE_BV[PKRU]=XINUSE[PKRU]=0 * update_pkru_in_sigframe() overwrites PKRU in XSAVE buffer ... signal handling * XRSTOR sees XSTATE_BV[PKRU]==0, ignores just-written value from update_pkru_in_sigframe() Fixes: 70044df ("x86/pkeys: Update PKRU to enable all pkeys before XSAVE") Suggested-by: Rudi Horn <[email protected]> Signed-off-by: Aruna Ramakrishna <[email protected]> Signed-off-by: Dave Hansen <[email protected]> Acked-by: Dave Hansen <[email protected]> Link: https://lore.kernel.org/all/20241119174520.3987538-3-aruna.ramakrishna%40oracle.com
1 parent 6a1853b commit ae6012d

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

arch/x86/kernel/fpu/xstate.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,22 @@ static inline u64 xfeatures_mask_independent(void)
7272
/*
7373
* Update the value of PKRU register that was already pushed onto the signal frame.
7474
*/
75-
static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u32 pkru)
75+
static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 mask, u32 pkru)
7676
{
77+
u64 xstate_bv;
78+
int err;
79+
7780
if (unlikely(!cpu_feature_enabled(X86_FEATURE_OSPKE)))
7881
return 0;
82+
83+
/* Mark PKRU as in-use so that it is restored correctly. */
84+
xstate_bv = (mask & xfeatures_in_use()) | XFEATURE_MASK_PKRU;
85+
86+
err = __put_user(xstate_bv, &buf->header.xfeatures);
87+
if (err)
88+
return err;
89+
90+
/* Update PKRU value in the userspace xsave buffer. */
7991
return __put_user(pkru, (unsigned int __user *)get_xsave_addr_user(buf, XFEATURE_PKRU));
8092
}
8193

@@ -292,7 +304,7 @@ static inline int xsave_to_user_sigframe(struct xregs_state __user *buf, u32 pkr
292304
clac();
293305

294306
if (!err)
295-
err = update_pkru_in_sigframe(buf, pkru);
307+
err = update_pkru_in_sigframe(buf, mask, pkru);
296308

297309
return err;
298310
}

0 commit comments

Comments
 (0)