KVM Archive on lore.kernel.org
 help / color / Atom feed
From: tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: riel@surriel.com, linux-kernel@vger.kernel.org, hpa@zytor.com,
	jannh@google.com, dave.hansen@intel.com, mingo@kernel.org,
	mingo@redhat.com, x86@kernel.org, tglx@linutronix.de,
	pbonzini@redhat.com, bigeasy@linutronix.de, Jason@zx2c4.com,
	luto@kernel.org, rkrcmar@redhat.com, kvm@vger.kernel.org,
	bp@suse.de
Subject: [tip:x86/fpu] x86/fpu: Remove fpu->initialized usage in __fpu__restore_sig()
Date: Sat, 13 Apr 2019 13:46:31 -0700
Message-ID: <tip-39ea9baffda91df8bfee9b45610242a3191ea1ec@git.kernel.org> (raw)
In-Reply-To: <20190403164156.19645-2-bigeasy@linutronix.de>

Commit-ID:  39ea9baffda91df8bfee9b45610242a3191ea1ec
Gitweb:     https://git.kernel.org/tip/39ea9baffda91df8bfee9b45610242a3191ea1ec
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 3 Apr 2019 18:41:30 +0200
Committer:  Borislav Petkov <bp@suse.de>
CommitDate: Tue, 9 Apr 2019 19:27:29 +0200

x86/fpu: Remove fpu->initialized usage in __fpu__restore_sig()

This is a preparation for the removal of the ->initialized member in the
fpu struct.

__fpu__restore_sig() is deactivating the FPU via fpu__drop() and then
setting manually ->initialized followed by fpu__restore(). The result is
that it is possible to manipulate fpu->state and the state of registers
won't be saved/restored on a context switch which would overwrite
fpu->state:

fpu__drop(fpu):
  ...
  fpu->initialized = 0;
  preempt_enable();

  <--- context switch

Don't access the fpu->state while the content is read from user space
and examined/sanitized. Use a temporary kmalloc() buffer for the
preparation of the FPU registers and once the state is considered okay,
load it. Should something go wrong, return with an error and without
altering the original FPU registers.

The removal of fpu__initialize() is a nop because fpu->initialized is
already set for the user task.

 [ bp: Massage a bit. ]

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Dave Hansen <dave.hansen@intel.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Borislav Petkov <bp@suse.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: kvm ML <kvm@vger.kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: x86-ml <x86@kernel.org>
Link: https://lkml.kernel.org/r/20190403164156.19645-2-bigeasy@linutronix.de
---
 arch/x86/include/asm/fpu/signal.h |  2 +-
 arch/x86/kernel/fpu/regset.c      |  5 ++---
 arch/x86/kernel/fpu/signal.c      | 40 +++++++++++++++------------------------
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/arch/x86/include/asm/fpu/signal.h b/arch/x86/include/asm/fpu/signal.h
index 44bbc39a57b3..7fb516b6893a 100644
--- a/arch/x86/include/asm/fpu/signal.h
+++ b/arch/x86/include/asm/fpu/signal.h
@@ -22,7 +22,7 @@ int ia32_setup_frame(int sig, struct ksignal *ksig,
 
 extern void convert_from_fxsr(struct user_i387_ia32_struct *env,
 			      struct task_struct *tsk);
-extern void convert_to_fxsr(struct task_struct *tsk,
+extern void convert_to_fxsr(struct fxregs_state *fxsave,
 			    const struct user_i387_ia32_struct *env);
 
 unsigned long
diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index bc02f5144b95..5dbc099178a8 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -269,11 +269,10 @@ convert_from_fxsr(struct user_i387_ia32_struct *env, struct task_struct *tsk)
 		memcpy(&to[i], &from[i], sizeof(to[0]));
 }
 
-void convert_to_fxsr(struct task_struct *tsk,
+void convert_to_fxsr(struct fxregs_state *fxsave,
 		     const struct user_i387_ia32_struct *env)
 
 {
-	struct fxregs_state *fxsave = &tsk->thread.fpu.state.fxsave;
 	struct _fpreg *from = (struct _fpreg *) &env->st_space[0];
 	struct _fpxreg *to = (struct _fpxreg *) &fxsave->st_space[0];
 	int i;
@@ -350,7 +349,7 @@ int fpregs_set(struct task_struct *target, const struct user_regset *regset,
 
 	ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1);
 	if (!ret)
-		convert_to_fxsr(target, &env);
+		convert_to_fxsr(&target->thread.fpu.state.fxsave, &env);
 
 	/*
 	 * update the header bit in the xsave header, indicating the
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 55b80de13ea5..7296a9bb78e7 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -207,11 +207,11 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
 }
 
 static inline void
-sanitize_restored_xstate(struct task_struct *tsk,
+sanitize_restored_xstate(union fpregs_state *state,
 			 struct user_i387_ia32_struct *ia32_env,
 			 u64 xfeatures, int fx_only)
 {
-	struct xregs_state *xsave = &tsk->thread.fpu.state.xsave;
+	struct xregs_state *xsave = &state->xsave;
 	struct xstate_header *header = &xsave->header;
 
 	if (use_xsave()) {
@@ -238,7 +238,7 @@ sanitize_restored_xstate(struct task_struct *tsk,
 		 */
 		xsave->i387.mxcsr &= mxcsr_feature_mask;
 
-		convert_to_fxsr(tsk, ia32_env);
+		convert_to_fxsr(&state->fxsave, ia32_env);
 	}
 }
 
@@ -284,8 +284,6 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 	if (!access_ok(buf, size))
 		return -EACCES;
 
-	fpu__initialize(fpu);
-
 	if (!static_cpu_has(X86_FEATURE_FPU))
 		return fpregs_soft_set(current, NULL,
 				       0, sizeof(struct user_i387_ia32_struct),
@@ -315,40 +313,32 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
 		 * header. Validate and sanitize the copied state.
 		 */
 		struct user_i387_ia32_struct env;
+		union fpregs_state *state;
 		int err = 0;
+		void *tmp;
 
-		/*
-		 * Drop the current fpu which clears fpu->initialized. This ensures
-		 * that any context-switch during the copy of the new state,
-		 * avoids the intermediate state from getting restored/saved.
-		 * Thus avoiding the new restored state from getting corrupted.
-		 * We will be ready to restore/save the state only after
-		 * fpu->initialized is again set.
-		 */
-		fpu__drop(fpu);
+		tmp = kzalloc(sizeof(*state) + fpu_kernel_xstate_size + 64, GFP_KERNEL);
+		if (!tmp)
+			return -ENOMEM;
+		state = PTR_ALIGN(tmp, 64);
 
 		if (using_compacted_format()) {
-			err = copy_user_to_xstate(&fpu->state.xsave, buf_fx);
+			err = copy_user_to_xstate(&state->xsave, buf_fx);
 		} else {
-			err = __copy_from_user(&fpu->state.xsave, buf_fx, state_size);
+			err = __copy_from_user(&state->xsave, buf_fx, state_size);
 
 			if (!err && state_size > offsetof(struct xregs_state, header))
-				err = validate_xstate_header(&fpu->state.xsave.header);
+				err = validate_xstate_header(&state->xsave.header);
 		}
 
 		if (err || __copy_from_user(&env, buf, sizeof(env))) {
-			fpstate_init(&fpu->state);
-			trace_x86_fpu_init_state(fpu);
 			err = -1;
 		} else {
-			sanitize_restored_xstate(tsk, &env, xfeatures, fx_only);
+			sanitize_restored_xstate(state, &env, xfeatures, fx_only);
+			copy_kernel_to_fpregs(state);
 		}
 
-		local_bh_disable();
-		fpu->initialized = 1;
-		fpu__restore(fpu);
-		local_bh_enable();
-
+		kfree(tmp);
 		return err;
 	} else {
 		/*

  reply index

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-03 16:41 [PATCH v9 00/27] x86: load FPU registers on return to userland Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 01/27] x86/fpu: Remove fpu->initialized usage in __fpu__restore_sig() Sebastian Andrzej Siewior
2019-04-13 20:46   ` tip-bot for Sebastian Andrzej Siewior [this message]
2019-04-03 16:41 ` [PATCH 02/27] x86/fpu: Remove fpu__restore() Sebastian Andrzej Siewior
2019-04-13 20:47   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 03/27] x86/fpu: Remove preempt_disable() in fpu__clear() Sebastian Andrzej Siewior
2019-04-13 20:48   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 04/27] x86/fpu: Always init the `state' " Sebastian Andrzej Siewior
2019-04-13 20:48   ` [tip:x86/fpu] x86/fpu: Always init the state " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 05/27] x86/fpu: Remove fpu->initialized usage in copy_fpstate_to_sigframe() Sebastian Andrzej Siewior
2019-04-13 20:49   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 06/27] x86/fpu: Don't save fxregs for ia32 frames " Sebastian Andrzej Siewior
2019-04-13 20:50   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 07/27] x86/fpu: Remove fpu->initialized Sebastian Andrzej Siewior
2019-04-13 20:50   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 08/27] x86/fpu: Remove user_fpu_begin() Sebastian Andrzej Siewior
2019-04-13 20:51   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 09/27] x86/fpu: Add (__)make_fpregs_active helpers Sebastian Andrzej Siewior
2019-04-13 20:52   ` [tip:x86/fpu] x86/fpu: Add an __fpregs_load_activate() internal helper tip-bot for Rik van Riel
2019-04-03 16:41 ` [PATCH 10/27] x86/fpu: Make __raw_xsave_addr() use feature number instead of mask Sebastian Andrzej Siewior
2019-04-13 20:52   ` [tip:x86/fpu] x86/fpu: Make __raw_xsave_addr() use a " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 11/27] x86/fpu: Make get_xsave_field_ptr() and get_xsave_addr() use " Sebastian Andrzej Siewior
2019-04-13 20:53   ` [tip:x86/fpu] x86/fpu: Use a feature number instead of mask in two more helpers tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 12/27] x86/pkru: Provide .*_pkru_ins() functions Sebastian Andrzej Siewior
2019-04-10 16:36   ` Borislav Petkov
2019-04-10 16:52     ` Borislav Petkov
2019-04-10 21:25       ` Sebastian Andrzej Siewior
2019-04-10 21:29         ` Dave Hansen
2019-04-11 13:24           ` Borislav Petkov
2019-04-13 20:54   ` [tip:x86/fpu] x86/pkeys: Provide *pkru() helpers tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 13/27] x86/fpu: Only write PKRU if it is different from current Sebastian Andrzej Siewior
2019-04-13 20:55   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 14/27] x86/pkeys: Don't check if PKRU is zero before writting it Sebastian Andrzej Siewior
2019-04-13 20:55   ` [tip:x86/fpu] x86/pkeys: Don't check if PKRU is zero before writing it tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 15/27] x86/fpu: Eager switch PKRU state Sebastian Andrzej Siewior
2019-04-13 20:56   ` [tip:x86/fpu] " tip-bot for Rik van Riel
2019-04-03 16:41 ` [PATCH 16/27] x86/entry: Add TIF_NEED_FPU_LOAD Sebastian Andrzej Siewior
2019-04-13 20:57   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 17/27] x86/fpu: Always store the registers in copy_fpstate_to_sigframe() Sebastian Andrzej Siewior
2019-04-13 20:57   ` [tip:x86/fpu] " tip-bot for Rik van Riel
2019-04-03 16:41 ` [PATCH 18/27] x86/fpu: Prepare copy_fpstate_to_sigframe() for TIF_NEED_FPU_LOAD Sebastian Andrzej Siewior
2019-04-13 20:58   ` [tip:x86/fpu] " tip-bot for Rik van Riel
2019-04-03 16:41 ` [PATCH 19/27] x86/fpu: Update xstate's PKRU value on write_pkru() Sebastian Andrzej Siewior
2019-04-08 18:14   ` Dave Hansen
2019-04-08 20:03     ` Sebastian Andrzej Siewior
2019-04-13 20:59   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 20/27] x86/fpu: Inline copy_user_to_fpregs_zeroing() Sebastian Andrzej Siewior
2019-04-13 21:00   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 21/27] x86/fpu: Let __fpu__restore_sig() restore the !32bit+fxsr frame from kernel memory Sebastian Andrzej Siewior
2019-04-13 21:00   ` [tip:x86/fpu] x86/fpu: Restore from kernel memory on the 64-bit path too tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 22/27] x86/fpu: Merge the two code paths in __fpu__restore_sig() Sebastian Andrzej Siewior
2019-04-13 21:01   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 23/27] x86/fpu: Defer FPU state load until return to userspace Sebastian Andrzej Siewior
2019-04-12 14:36   ` Borislav Petkov
2019-04-12 15:24     ` Sebastian Andrzej Siewior
2019-04-12 16:22       ` Borislav Petkov
2019-04-12 16:37         ` Sebastian Andrzej Siewior
2019-04-12 16:48           ` Borislav Petkov
2019-04-12 17:19             ` Sebastian Andrzej Siewior
2019-04-12 17:29               ` Borislav Petkov
2019-04-15  9:14                 ` Sebastian Andrzej Siewior
2019-04-13 21:02   ` [tip:x86/fpu] " tip-bot for Rik van Riel
2019-04-03 16:41 ` [PATCH 24/27] x86/fpu: Add a fastpath to __fpu__restore_sig() Sebastian Andrzej Siewior
2019-04-08 17:05   ` Thomas Gleixner
2019-04-08 20:02     ` Sebastian Andrzej Siewior
2019-04-09  7:27       ` Thomas Gleixner
2019-04-12 17:17   ` Borislav Petkov
2019-04-12 17:27     ` Sebastian Andrzej Siewior
2019-04-13 21:02   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 25/27] x86/fpu: Add a fastpath to copy_fpstate_to_sigframe() Sebastian Andrzej Siewior
2019-04-13 21:03   ` [tip:x86/fpu] " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 26/27] x86/fpu: Restore FPU register in copy_fpstate_to_sigframe() in order to use the fastpath Sebastian Andrzej Siewior
2019-04-13 21:04   ` [tip:x86/fpu] x86/fpu: Restore regs " tip-bot for Sebastian Andrzej Siewior
2019-04-03 16:41 ` [PATCH 27/27] x86/pkeys: add PKRU value to init_fpstate Sebastian Andrzej Siewior
2019-04-13 21:05   ` [tip:x86/fpu] x86/pkeys: Add " tip-bot for Sebastian Andrzej Siewior
2019-04-04 14:01 ` [PATCH v9 00/27] x86: load FPU registers on return to userland David Laight
2019-04-04 14:14   ` 'Sebastian Andrzej Siewior'
2019-04-04 14:26     ` Andy Lutomirski
2019-04-04 14:31       ` Sebastian Andrzej Siewior
2019-04-04 15:10       ` David Laight
2019-04-08 17:08 ` Thomas Gleixner
2019-04-12 18:30 ` Borislav Petkov
2019-04-15  8:58   ` Sebastian Andrzej Siewior

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=tip-39ea9baffda91df8bfee9b45610242a3191ea1ec@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=Jason@zx2c4.com \
    --cc=bigeasy@linutronix.de \
    --cc=bp@suse.de \
    --cc=dave.hansen@intel.com \
    --cc=hpa@zytor.com \
    --cc=jannh@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=riel@surriel.com \
    --cc=rkrcmar@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /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

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org
	public-inbox-index kvm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git