Skip to contentSkip to conversation

FAILED: patch "[PATCH] x86/fpu: Stop relying on userspace for info to fault in xsave" failed to apply to 5.15-stable tree

PATCH In review [email protected] 2 participants 4 messages Feb 19, 2024 – Feb 25, 2024

Subsystems (2)
Stable branch x86
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 5.15-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x d877550eaf2dc9090d782864c96939397a3c6835
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021941-reprimand-grudge-7734@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..

Possible dependencies:

d877550eaf2d ("x86/fpu: Stop relying on userspace for info to fault in xsave buffer")
c03098d4b9ad ("Merge tag 'gfs2-v5.15-rc5-mmap-fault' of git://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From d877550eaf2dc9090d782864c96939397a3c6835 Mon Sep 17 00:00:00 2001
From: Andrei Vagin <[email protected]>
Date: Mon, 29 Jan 2024 22:36:03 -0800
Subject: [PATCH] x86/fpu: Stop relying on userspace for info to fault in xsave
buffer

Before this change, the expected size of the user space buffer was taken from fx_sw->xstate_size. fx_sw->xstate_size can be changed from user-space, so it is possible construct a sigreturn frame where:

* fx_sw->xstate_size is smaller than the size required by valid bits in fx_sw->xfeatures.
* user-space unmaps parts of the sigrame fpu buffer so that not all of the buffer required by xrstor is accessible.

In this case, xrstor tries to restore and accesses the unmapped area which results in a fault. But fault_in_readable succeeds because buf + fx_sw->xstate_size is within the still mapped area, so it goes back and tries xrstor again. It will spin in this loop forever.

Instead, fault in the maximum size which can be touched by XRSTOR (taken from fpstate->user_size).

[ dhansen: tweak subject / changelog ]

Fixes fcb3635f5018 ("x86/fpu/signal: Handle #PF in the direct restore path")
Reported-by Konstantin Bogomolov <[email protected]>
Suggested-by Thomas Gleixner <[email protected]>
Signed-off-by Andrei Vagin <[email protected]>
Signed-off-by Dave Hansen <[email protected]>
Cc [email protected]
Link https://lore.kernel.org/all/20240130063603.3392627-1-avagin%40google.com open here
arch/x86/kernel/fpu/signal.c +5 -8
index 558076dbde5b..247f2225aa9f 100644
@@ -274,12 +274,13 @@ static int __restore_fpregs_from_user(void __user *buf, u64 ufeatures,
  * Attempt to restore the FPU registers directly from user memory.
  * Pagefaults are handled and any errors returned are fatal.
  */
-static bool restore_fpregs_from_user(void __user *buf, u64 xrestore,
-				     bool fx_only, unsigned int size)
+static bool restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only)
 {
 	struct fpu *fpu = &current->thread.fpu;
 	int ret;
 
+	/* Restore enabled features only. */
+	xrestore &= fpu->fpstate->user_xfeatures;
 retry:
 	fpregs_lock();
 	/* Ensure that XFD is up to date */

@@ -309,7 +310,7 @@ static bool restore_fpregs_from_user(void __user *buf, u64 xrestore,
 		if (ret != X86_TRAP_PF)
 			return false;
 
-		if (!fault_in_readable(buf, size))
+		if (!fault_in_readable(buf, fpu->fpstate->user_size))
 			goto retry;
 		return false;
 	}

@@ -339,7 +340,6 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
 	struct user_i387_ia32_struct env;
 	bool success, fx_only = false;
 	union fpregs_state *fpregs;
-	unsigned int state_size;
 	u64 user_xfeatures = 0;
 
 	if (use_xsave()) {

@@ -349,17 +349,14 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
 			return false;
 
 		fx_only = !fx_sw_user.magic1;
-		state_size = fx_sw_user.xstate_size;
 		user_xfeatures = fx_sw_user.xfeatures;
 	} else {
 		user_xfeatures = XFEATURE_MASK_FPSSE;
-		state_size = fpu->fpstate->user_size;
 	}
 
 	if (likely(!ia32_fxstate)) {
 		/* Restore the FPU registers directly from user memory. */
-		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only,
-						state_size);
+		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only);
 	}
 
 	/*
#1 lore raw

PATCH 1/1 x86/fpu: Stop relying on userspace for info to fault in xsave

From: Andrei Vagin <[email protected]>

Before this change, the expected size of the user space buffer was taken from fx_sw->xstate_size. fx_sw->xstate_size can be changed from user-space, so it is possible construct a sigreturn frame where:

* fx_sw->xstate_size is smaller than the size required by valid bits in fx_sw->xfeatures.
* user-space unmaps parts of the sigrame fpu buffer so that not all of the buffer required by xrstor is accessible.

In this case, xrstor tries to restore and accesses the unmapped area which results in a fault. But fault_in_readable succeeds because buf + fx_sw->xstate_size is within the still mapped area, so it goes back and tries xrstor again. It will spin in this loop forever.

Instead, fault in the maximum size which can be touched by XRSTOR (taken from fpstate->user_size).

[ dhansen: tweak subject / changelog ]
[ tglx: Backport to 5.15 stable ]

Fixes fcb3635f5018 ("x86/fpu/signal: Handle #PF in the direct restore path")
Reported-by Konstantin Bogomolov <[email protected]>
Suggested-by Thomas Gleixner <[email protected]>
Signed-off-by Andrei Vagin <[email protected]>
Signed-off-by Dave Hansen <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Link https://lore.kernel.org/all/20240130063603.3392627-1-avagin%40google.com open here
---
 arch/x86/kernel/fpu/signal.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)
arch/x86/kernel/fpu/signal.c +5 -7
@@ -246,12 +246,13 @@ static int __restore_fpregs_from_user(vo
  * Attempt to restore the FPU registers directly from user memory.
  * Pagefaults are handled and any errors returned are fatal.
  */
-static int restore_fpregs_from_user(void __user *buf, u64 xrestore,
-				    bool fx_only, unsigned int size)
+static int restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only)
 {
 	struct fpu *fpu = &current->thread.fpu;
 	int ret;
 
+	/* Restore enabled features only. */
+	xrestore &= xfeatures_mask_all & XFEATURE_MASK_USER_SUPPORTED;
 retry:
 	fpregs_lock();
 	pagefault_disable();

@@ -278,7 +279,7 @@ static int restore_fpregs_from_user(void
 		if (ret != -EFAULT)
 			return -EINVAL;
 
-		if (!fault_in_readable(buf, size))
+		if (!fault_in_readable(buf, fpu_user_xstate_size))
 			goto retry;
 		return -EFAULT;
 	}

@@ -303,7 +304,6 @@ static int restore_fpregs_from_user(void
 static int __fpu_restore_sig(void __user *buf, void __user *buf_fx,
 			     bool ia32_fxstate)
 {
-	int state_size = fpu_kernel_xstate_size;
 	struct task_struct *tsk = current;
 	struct fpu *fpu = &tsk->thread.fpu;
 	struct user_i387_ia32_struct env;

@@ -319,7 +319,6 @@ static int __fpu_restore_sig(void __user
 			return ret;
 
 		fx_only = !fx_sw_user.magic1;
-		state_size = fx_sw_user.xstate_size;
 		user_xfeatures = fx_sw_user.xfeatures;
 	} else {
 		user_xfeatures = XFEATURE_MASK_FPSSE;

@@ -332,8 +331,7 @@ static int __fpu_restore_sig(void __user
 		 * faults. If it does, fall back to the slow path below, going
 		 * through the kernel buffer with the enabled pagefault handler.
 		 */
-		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only,
-						state_size);
+		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only);
 	}
 
 	/*
author maintainer
#2 lore raw

Re: [PATCH linux-5.15.y] x86/fpu: Stop relying on userspace for info to fault in xsave

Quoted 30 lines · Thomas Gleixner

From: Andrei Vagin <[email protected]>

Before this change, the expected size of the user space buffer was taken from fx_sw->xstate_size. fx_sw->xstate_size can be changed from user-space, so it is possible construct a sigreturn frame where:

* fx_sw->xstate_size is smaller than the size required by valid bits in fx_sw->xfeatures.
* user-space unmaps parts of the sigrame fpu buffer so that not all of the buffer required by xrstor is accessible.

In this case, xrstor tries to restore and accesses the unmapped area which results in a fault. But fault_in_readable succeeds because buf + fx_sw->xstate_size is within the still mapped area, so it goes back and tries xrstor again. It will spin in this loop forever.

Instead, fault in the maximum size which can be touched by XRSTOR (taken from fpstate->user_size).

[ dhansen: tweak subject / changelog ]
[ tglx: Backport to 5.15 stable ]

Fixes fcb3635f5018 ("x86/fpu/signal: Handle #PF in the direct restore path")
Reported-by Konstantin Bogomolov <[email protected]>
Suggested-by Thomas Gleixner <[email protected]>
Signed-off-by Andrei Vagin <[email protected]>
Signed-off-by Dave Hansen <[email protected]>
Signed-off-by Thomas Gleixner <[email protected]>
Link https://lore.kernel.org/all/20240130063603.3392627-1-avagin%40google.com open here
Quoted 3 lines
---
 arch/x86/kernel/fpu/signal.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

Nit, you forgot to give me a hint what the git id was :(

I figured it out, now queued up, thanks.

greg k-h

#3 lore raw

Re: [PATCH linux-5.15.y] x86/fpu: Stop relying on userspace for info to fault in xsave

Quoted 1 line · Greg KH

Nit, you forgot to give me a hint what the git id was :(

Ooops. Sorry.

Quoted 1 line

I figured it out, now queued up, thanks.

Thank you!

Thomas