Skip to content

Commit abc790b

Browse files
KAGA-KOKOgregkh
authored andcommitted
x86/pkru: Write hardware init value to PKRU when xstate is init
commit 510b80a upstream. When user space brings PKRU into init state, then the kernel handling is broken: T1 user space xsave(state) state.header.xfeatures &= ~XFEATURE_MASK_PKRU; xrstor(state) T1 -> kernel schedule() XSAVE(S) -> T1->xsave.header.xfeatures[PKRU] == 0 T1->flags |= TIF_NEED_FPU_LOAD; wrpkru(); schedule() ... pk = get_xsave_addr(&T1->fpu->state.xsave, XFEATURE_PKRU); if (pk) wrpkru(pk->pkru); else wrpkru(DEFAULT_PKRU); Because the xfeatures bit is 0 and therefore the value in the xsave storage is not valid, get_xsave_addr() returns NULL and switch_to() writes the default PKRU. -> FAIL xen-troops#1! So that wrecks any copy_to/from_user() on the way back to user space which hits memory which is protected by the default PKRU value. Assumed that this does not fail (pure luck) then T1 goes back to user space and because TIF_NEED_FPU_LOAD is set it ends up in switch_fpu_return() __fpregs_load_activate() if (!fpregs_state_valid()) { load_XSTATE_from_task(); } But if nothing touched the FPU between T1 scheduling out and back in, then the fpregs_state is still valid which means switch_fpu_return() does nothing and just clears TIF_NEED_FPU_LOAD. Back to user space with DEFAULT_PKRU loaded. -> FAIL xen-troops#2! The fix is simple: if get_xsave_addr() returns NULL then set the PKRU value to 0 instead of the restrictive default PKRU value in init_pkru_value. [ bp: Massage in minor nitpicks from folks. ] Fixes: 0cecca9 ("x86/fpu: Eager switch PKRU state") Signed-off-by: Thomas Gleixner <[email protected]> Signed-off-by: Borislav Petkov <[email protected]> Acked-by: Dave Hansen <[email protected]> Acked-by: Rik van Riel <[email protected]> Tested-by: Babu Moger <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 208bb68 commit abc790b

File tree

1 file changed

+9
-2
lines changed

1 file changed

+9
-2
lines changed

arch/x86/include/asm/fpu/internal.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,9 +579,16 @@ static inline void switch_fpu_finish(struct fpu *new_fpu)
579579
* return to userland e.g. for a copy_to_user() operation.
580580
*/
581581
if (!(current->flags & PF_KTHREAD)) {
582+
/*
583+
* If the PKRU bit in xsave.header.xfeatures is not set,
584+
* then the PKRU component was in init state, which means
585+
* XRSTOR will set PKRU to 0. If the bit is not set then
586+
* get_xsave_addr() will return NULL because the PKRU value
587+
* in memory is not valid. This means pkru_val has to be
588+
* set to 0 and not to init_pkru_value.
589+
*/
582590
pk = get_xsave_addr(&new_fpu->state.xsave, XFEATURE_PKRU);
583-
if (pk)
584-
pkru_val = pk->pkru;
591+
pkru_val = pk ? pk->pkru : 0;
585592
}
586593
__write_pkru(pkru_val);
587594
}

0 commit comments

Comments
 (0)