FAILED: patch "[PATCH] x86/ptrace: Fix xfpregs_set()'s incorrect xmm clearing" failed to apply to 5.15-stable tree

PATCH [email protected] 2 participants 3 messages Feb 21, 2022 – Feb 23, 2022

Subsystems (3)

Stable branch x86 ptrace support

lore

Share & tools

lore

[email protected]

author maintainer

2022-02-21 06:57 UTC #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]>.

thanks,

greg k-h

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

From 44cad52cc14ae10062f142ec16ede489bccf4469 Mon Sep 17 00:00:00 2001
From: Andy Lutomirski <[email protected]>
Date: Mon, 14 Feb 2022 13:05:49 +0100
Subject: [PATCH] x86/ptrace: Fix xfpregs_set()'s incorrect xmm clearing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

xfpregs_set() handles 32-bit REGSET_XFP and 64-bit REGSET_FP. The actual code treats these regsets as modern FX state (i.e. the beginning part of XSTATE). The declarations of the regsets thought they were the legacy i387 format. The code thought they were the 32-bit (no xmm8..15) variant of XSTATE and, for good measure, made the high bits disappear by zeroing the wrong part of the buffer. The latter broke ptrace, and everything else confused anyone trying to understand the code. In particular, the nonsense definitions of the regsets confused me when I wrote this code.

Clean this all up. Change the declarations to match reality (which shouldn't change the generated code, let alone the ABI) and fix xfpregs_set() to clear the correct bits and to only do so for 32-bit callers.

Fixes 6164331d15f7 ("x86/fpu: Rewrite xfpregs_set()")

Reported-by Luís Ferreira <[email protected]>

Signed-off-by Andy Lutomirski <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

Cc <[email protected]>

Link https://bugzilla.kernel.org/show_bug.cgi?id=215524

Link https://lore.kernel.org/r/[email protected] open here

arch/x86/kernel/fpu/regset.c +4 -5

index 437d7c930c0b..75ffaef8c299 100644
@@ -91,11 +91,9 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 		const void *kbuf, const void __user *ubuf)
 {
 	struct fpu *fpu = &target->thread.fpu;
-	struct user32_fxsr_struct newstate;
+	struct fxregs_state newstate;
 	int ret;
 
-	BUILD_BUG_ON(sizeof(newstate) != sizeof(struct fxregs_state));
-
 	if (!cpu_feature_enabled(X86_FEATURE_FXSR))
 		return -ENODEV;

@@ -116,9 +114,10 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 	/* Copy the state  */
 	memcpy(&fpu->fpstate->regs.fxsave, &newstate, sizeof(newstate));
 
-	/* Clear xmm8..15 */
+	/* Clear xmm8..15 for 32-bit callers */
 	BUILD_BUG_ON(sizeof(fpu->__fpstate.regs.fxsave.xmm_space) != 16 * 16);
-	memset(&fpu->fpstate->regs.fxsave.xmm_space[8], 0, 8 * 16);
+	if (in_ia32_syscall())
+		memset(&fpu->fpstate->regs.fxsave.xmm_space[8*4], 0, 8 * 16);
 
 	/* Mark FP and SSE as in use when XSAVE is enabled */
 	if (use_xsave())

arch/x86/kernel/ptrace.c +2 -2

index 6d2244c94799..8d2f2f995539 100644
@@ -1224,7 +1224,7 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
 	},
 	[REGSET_FP] = {
 		.core_note_type = NT_PRFPREG,
-		.n = sizeof(struct user_i387_struct) / sizeof(long),
+		.n = sizeof(struct fxregs_state) / sizeof(long),
 		.size = sizeof(long), .align = sizeof(long),
 		.active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
 	},

@@ -1271,7 +1271,7 @@ static struct user_regset x86_32_regsets[] __ro_after_init = {
 	},
 	[REGSET_XFP] = {
 		.core_note_type = NT_PRXFPREG,
-		.n = sizeof(struct user32_fxsr_struct) / sizeof(u32),
+		.n = sizeof(struct fxregs_state) / sizeof(u32),
 		.size = sizeof(u32), .align = sizeof(u32),
 		.active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
 	},

Borislav Petkov

2022-02-21 09:22 UTC #1 lore raw

Quoted 5 lines · On Mon, Feb 21, 2022 at 07:57:39AM +0100, [email protected] wrote:

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]>.

---
From b0535322d006c7f49e7fca3485991c5f88a5e7cb Mon Sep 17 00:00:00 2001
From: Andy Lutomirski <[email protected]>
Date: Mon, 14 Feb 2022 13:05:49 +0100
Subject: [PATCH] x86/ptrace: Fix xfpregs_set()'s incorrect xmm clearing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Commit 44cad52cc14ae10062f142ec16ede489bccf4469 upstream.

Fixes 6164331d15f7 ("x86/fpu: Rewrite xfpregs_set()")

Reported-by Luís Ferreira <[email protected]>

Signed-off-by Andy Lutomirski <[email protected]>

Signed-off-by Borislav Petkov <[email protected]>

Cc <[email protected]>

Link https://bugzilla.kernel.org/show_bug.cgi?id=215524

Link https://lore.kernel.org/r/[email protected] open here

---
 arch/x86/kernel/fpu/regset.c | 9 ++++-----
 arch/x86/kernel/ptrace.c     | 4 ++--
 2 files changed, 6 insertions(+), 7 deletions(-)

arch/x86/kernel/fpu/regset.c +4 -5

index 66ed317ebc0d..125cbbe10fef 100644
@@ -87,11 +87,9 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 		const void *kbuf, const void __user *ubuf)
 {
 	struct fpu *fpu = &target->thread.fpu;
-	struct user32_fxsr_struct newstate;
+	struct fxregs_state newstate;
 	int ret;
 
-	BUILD_BUG_ON(sizeof(newstate) != sizeof(struct fxregs_state));
-
 	if (!cpu_feature_enabled(X86_FEATURE_FXSR))
 		return -ENODEV;

@@ -112,9 +110,10 @@ int xfpregs_set(struct task_struct *target, const struct user_regset *regset,
 	/* Copy the state  */
 	memcpy(&fpu->state.fxsave, &newstate, sizeof(newstate));
 
-	/* Clear xmm8..15 */
+	/* Clear xmm8..15 for 32-bit callers */
 	BUILD_BUG_ON(sizeof(fpu->state.fxsave.xmm_space) != 16 * 16);
-	memset(&fpu->state.fxsave.xmm_space[8], 0, 8 * 16);
+	if (in_ia32_syscall())
+		memset(&fpu->state.fxsave.xmm_space[8*4], 0, 8 * 16);
 
 	/* Mark FP and SSE as in use when XSAVE is enabled */
 	if (use_xsave())

arch/x86/kernel/ptrace.c +2 -2

index 4c208ea3bd9f..033d9c6a9468 100644
@@ -1224,7 +1224,7 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
 	},
 	[REGSET_FP] = {
 		.core_note_type = NT_PRFPREG,
-		.n = sizeof(struct user_i387_struct) / sizeof(long),
+		.n = sizeof(struct fxregs_state) / sizeof(long),
 		.size = sizeof(long), .align = sizeof(long),
 		.active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
 	},

@@ -1271,7 +1271,7 @@ static struct user_regset x86_32_regsets[] __ro_after_init = {
 	},
 	[REGSET_XFP] = {
 		.core_note_type = NT_PRXFPREG,
-		.n = sizeof(struct user32_fxsr_struct) / sizeof(u32),
+		.n = sizeof(struct fxregs_state) / sizeof(u32),
 		.size = sizeof(u32), .align = sizeof(u32),
 		.active = regset_xregset_fpregs_active, .regset_get = xfpregs_get, .set = xfpregs_set
 	},

Signature · 7 lines

2.29.2

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Ivo Totev, HRB 36809, AG Nürnberg

Greg KH

author maintainer

2022-02-23 18:18 UTC #2 lore raw

Quoted 39 lines · Borislav Petkov

Quoted 5 lines · On Mon, Feb 21, 2022 at 07:57:39AM +0100, [email protected] wrote:
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]>.
---
Quoted 1 line
From b0535322d006c7f49e7fca3485991c5f88a5e7cb Mon Sep 17 00:00:00 2001
From: Andy Lutomirski <[email protected]>
Date: Mon, 14 Feb 2022 13:05:49 +0100
Subject: [PATCH] x86/ptrace: Fix xfpregs_set()'s incorrect xmm clearing
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 44cad52cc14ae10062f142ec16ede489bccf4469 upstream.
xfpregs_set() handles 32-bit REGSET_XFP and 64-bit REGSET_FP. The actual code treats these regsets as modern FX state (i.e. the beginning part of XSTATE). The declarations of the regsets thought they were the legacy i387 format. The code thought they were the 32-bit (no xmm8..15) variant of XSTATE and, for good measure, made the high bits disappear by zeroing the wrong part of the buffer. The latter broke ptrace, and everything else confused anyone trying to understand the code. In particular, the nonsense definitions of the regsets confused me when I wrote this code.
Clean this all up. Change the declarations to match reality (which shouldn't change the generated code, let alone the ABI) and fix xfpregs_set() to clear the correct bits and to only do so for 32-bit callers.
Fixes 6164331d15f7 ("x86/fpu: Rewrite xfpregs_set()")
Reported-by Luís Ferreira <[email protected]>
Signed-off-by Andy Lutomirski <[email protected]>
Signed-off-by Borislav Petkov <[email protected]>
Cc <[email protected]>
Link https://bugzilla.kernel.org/show_bug.cgi?id=215524
Link https://lore.kernel.org/r/[email protected] open here

Quoted 4 lines

---
 arch/x86/kernel/fpu/regset.c | 9 ++++-----
 arch/x86/kernel/ptrace.c     | 4 ++--
 2 files changed, 6 insertions(+), 7 deletions(-)

Now queued up,t hanks.

greg k-h