linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Andy Lutomirski <luto@kernel.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Tony Luck <tony.luck@intel.com>,
	Yu-cheng Yu <yu-cheng.yu@intel.com>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Rik van Riel <riel@surriel.com>, Borislav Petkov <bp@suse.de>
Subject: [patch V3 4/6] x86/pkru: Make PKRU=0 actually work
Date: Tue, 08 Jun 2021 16:36:21 +0200	[thread overview]
Message-ID: <20210608144346.045616965@linutronix.de> (raw)
In-Reply-To: 20210608143617.565868844@linutronix.de

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 #1!

So that wreckages 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 in the
fpregs_state is valid so switch_fpu_return() does nothing and just clears
TIF_NEED_FPU_LOAD. Back to user space with DEFAULT_PKRU loaded. -> FAIL #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.

Fixes: 0cecca9d03c9 ("x86/fpu: Eager switch PKRU state")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Rik van Riel <riel@surriel.com>
Cc: stable@vger.kernel.org
---
 arch/x86/include/asm/fpu/internal.h |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -579,9 +579,16 @@ static inline void switch_fpu_finish(str
 	 * return to userland e.g. for a copy_to_user() operation.
 	 */
 	if (!(current->flags & PF_KTHREAD)) {
+		/*
+		 * If the PKRU bit in xsave.header.xfeatures is not set,
+		 * then the PKRU compoment was in init state, which means
+		 * XRSTOR will set PKRU to 0. If the bit is not set then
+		 * get_xsave_addr() will return NULL because the PKRU value
+		 * in memory is not valid. This means pkru_val has to be
+		 * set to 0 and not to init_pkru_value.
+		 */
 		pk = get_xsave_addr(&new_fpu->state.xsave, XFEATURE_PKRU);
-		if (pk)
-			pkru_val = pk->pkru;
+		pkru_val = pk ? pk->pkru : 0;
 	}
 	__write_pkru(pkru_val);
 }


  parent reply	other threads:[~2021-06-08 14:49 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08 14:36 [patch V3 0/6] x86/fpu: Mop up XSAVES and related damage Thomas Gleixner
2021-06-08 14:36 ` [patch V3 1/6] x86/fpu: Prevent state corruption in __fpu__restore_sig() Thomas Gleixner
2021-06-09 14:46   ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2021-06-08 14:36 ` [patch V3 2/6] x86/fpu: Invalidate FPU state after a failed XRSTOR from a user buffer Thomas Gleixner
2021-06-09 14:46   ` [tip: x86/urgent] " tip-bot2 for Andy Lutomirski
2021-06-08 14:36 ` [patch V3 3/6] x86/process: Check PF_KTHREAD and not current->mm for kernel threads Thomas Gleixner
2021-06-09 14:46   ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2021-06-10 17:10   ` [patch V3 3/6] " Andy Lutomirski
2021-06-10 20:54     ` Thomas Gleixner
2021-06-11  1:04       ` Andy Lutomirski
2021-06-08 14:36 ` Thomas Gleixner [this message]
2021-06-08 15:40   ` [patch V3 4/6] x86/pkru: Make PKRU=0 actually work Borislav Petkov
2021-06-08 19:15     ` Thomas Gleixner
2021-06-08 20:06       ` Borislav Petkov
2021-06-08 16:06   ` Dave Hansen
2021-06-08 19:06     ` Thomas Gleixner
2021-06-08 21:37   ` Babu Moger
2021-06-09 14:46   ` [tip: x86/urgent] x86/pkru: Write hardware init value to PKRU when xstate is init tip-bot2 for Thomas Gleixner
2021-06-08 14:36 ` [patch V3 5/6] x86/fpu: Add address range checks to copy_user_to_xstate() Thomas Gleixner
2021-06-09 12:56   ` [tip: x86/fpu] " tip-bot2 for Andy Lutomirski
2021-06-08 14:36 ` [patch V3 6/6] selftests/x86: Test signal frame XSTATE header corruption handling Thomas Gleixner
2021-06-09  8:38   ` David Edmondson
2021-06-09 12:56   ` [tip: x86/fpu] " tip-bot2 for Andy Lutomirski
2021-06-08 16:08 ` [patch V3 0/6] x86/fpu: Mop up XSAVES and related damage Dave Hansen
2021-06-08 18:46 ` Rik van Riel
2021-06-09 19:18 ` [PATCH] x86/fpu: Reset state for all signal restore failures Thomas Gleixner
2021-06-10  6:39   ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210608144346.045616965@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=bigeasy@linutronix.de \
    --cc=bp@suse.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=riel@surriel.com \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=yu-cheng.yu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).