um: always use the internal copy of the FP registers

When switching from userspace to the kernel, all registers including the FP registers are copied into the kernel and restored later on. As such, the true source for the FP register state is actually already in the kernel and they should never be grabbed from the userspace process. Change the various places to simply copy the data from the internal FP register storage area. Note that on i386 the format of PTRACE_GETFPREGS and PTRACE_GETFPXREGS is different enough that conversion would be needed. With this patch, -EINVAL is returned if the non-native format is requested. The upside is, that this patchset fixes setting registers via ptrace (which simply did not work before) as well as fixing setting floating point registers using the mcontext on signal return on i386. Signed-off-by: Benjamin Berg <benjamin.berg@intel.com> Link: https://patch.msgid.link/20240913133845.964292-1-benjamin@sipsolutions.net Signed-off-by: Johannes Berg <johannes.berg@intel.com>
author: Benjamin Berg <benjamin.berg@intel.com> 2024-09-13 15:38:45 +0200
committer: Johannes Berg <johannes.berg@intel.com> 2024-10-10 12:03:55 +0200
commit: 5a6951273e0e9dc0f79facf22281a8a731fb90b1 (patch)
tree: e8b65704dc743bcfd211950bd7015f7972b745a4
parent: 865e3845eeaa21e9a62abc1361644e67124f1ec0 (diff)
download: lwn-5a6951273e0e9dc0f79facf22281a8a731fb90b1.tar.gz
lwn-5a6951273e0e9dc0f79facf22281a8a731fb90b1.zip
6 files changed, 80 insertions, 84 deletions
diff --git a/arch/um/include/shared/registers.h b/arch/um/include/shared/registers.h
index a0450326521c..7d81b2339a48 100644
--- a/arch/um/include/shared/registers.h
+++ b/arch/um/include/shared/registers.h
@@ -8,12 +8,6 @@
 
 #include <sysdep/ptrace.h>
 
-extern int save_i387_registers(int pid, unsigned long *fp_regs);
-extern int restore_i387_registers(int pid, unsigned long *fp_regs);
-extern int save_fp_registers(int pid, unsigned long *fp_regs);
-extern int restore_fp_registers(int pid, unsigned long *fp_regs);
-extern int save_fpx_registers(int pid, unsigned long *fp_regs);
-extern int restore_fpx_registers(int pid, unsigned long *fp_regs);
 extern int init_pid_registers(int pid);
 extern void get_safe_registers(unsigned long *regs, unsigned long *fp_regs);
 extern int get_fp_registers(int pid, unsigned long *regs);
diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
index 9c6cf03ed02b..701b7bb2525e 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -290,8 +290,15 @@ unsigned long __get_wchan(struct task_struct *p)
 
 int elf_core_copy_task_fpregs(struct task_struct *t, elf_fpregset_t *fpu)
 {
-	int cpu = current_thread_info()->cpu;
+#ifdef CONFIG_X86_32
+	extern int have_fpx_regs;
 
-	return save_i387_registers(userspace_pid[cpu], (unsigned long *) fpu) == 0;
+	/* FIXME: A plain copy does not work on i386 with have_fpx_regs */
+	if (have_fpx_regs)
+		return 0;
+#endif
+	memcpy(fpu, &t->thread.regs.regs.fp, sizeof(*fpu));
+
+	return 1;
 }
 
diff --git a/arch/x86/um/os-Linux/registers.c b/arch/x86/um/os-Linux/registers.c
index f3638dd09cec..f8e5516d9708 100644
--- a/arch/x86/um/os-Linux/registers.c
+++ b/arch/x86/um/os-Linux/registers.c
@@ -19,14 +19,14 @@
 
 static int have_xstate_support;
 
-int save_i387_registers(int pid, unsigned long *fp_regs)
+static int save_i387_registers(int pid, unsigned long *fp_regs)
 {
 	if (ptrace(PTRACE_GETFPREGS, pid, 0, fp_regs) < 0)
 		return -errno;
 	return 0;
 }
 
-int save_fp_registers(int pid, unsigned long *fp_regs)
+static int save_fp_registers(int pid, unsigned long *fp_regs)
 {
 #ifdef PTRACE_GETREGSET
 	struct iovec iov;
@@ -42,14 +42,14 @@ int save_fp_registers(int pid, unsigned long *fp_regs)
 		return save_i387_registers(pid, fp_regs);
 }
 
-int restore_i387_registers(int pid, unsigned long *fp_regs)
+static int restore_i387_registers(int pid, unsigned long *fp_regs)
 {
 	if (ptrace(PTRACE_SETFPREGS, pid, 0, fp_regs) < 0)
 		return -errno;
 	return 0;
 }
 
-int restore_fp_registers(int pid, unsigned long *fp_regs)
+static int restore_fp_registers(int pid, unsigned long *fp_regs)
 {
 #ifdef PTRACE_SETREGSET
 	struct iovec iov;
@@ -66,14 +66,14 @@ int restore_fp_registers(int pid, unsigned long *fp_regs)
 
 #ifdef __i386__
 int have_fpx_regs = 1;
-int save_fpx_registers(int pid, unsigned long *fp_regs)
+static int save_fpx_registers(int pid, unsigned long *fp_regs)
 {
 	if (ptrace(PTRACE_GETFPXREGS, pid, 0, fp_regs) < 0)
 		return -errno;
 	return 0;
 }
 
-int restore_fpx_registers(int pid, unsigned long *fp_regs)
+static int restore_fpx_registers(int pid, unsigned long *fp_regs)
 {
 	if (ptrace(PTRACE_SETFPXREGS, pid, 0, fp_regs) < 0)
 		return -errno;
diff --git a/arch/x86/um/ptrace_32.c b/arch/x86/um/ptrace_32.c
index b0a71c6cdc6e..b8b85a52eb6f 100644
--- a/arch/x86/um/ptrace_32.c
+++ b/arch/x86/um/ptrace_32.c
@@ -168,17 +168,18 @@ int peek_user(struct task_struct *child, long addr, long data)
 	return put_user(tmp, (unsigned long __user *) data);
 }
 
+/* FIXME: Do the required conversions instead of erroring out */
+extern int have_fpx_regs;
+
 static int get_fpregs(struct user_i387_struct __user *buf, struct task_struct *child)
 {
-	int err, n, cpu = task_cpu(child);
-	struct user_i387_struct fpregs;
+	int n;
 
-	err = save_i387_registers(userspace_pid[cpu],
-				  (unsigned long *) &fpregs);
-	if (err)
-		return err;
+	if (have_fpx_regs)
+		return -EINVAL;
 
-	n = copy_to_user(buf, &fpregs, sizeof(fpregs));
+	n = copy_to_user(buf, &child->thread.regs.regs.fp,
+			 sizeof(struct user_i387_struct));
 	if(n > 0)
 		return -EFAULT;
 
@@ -187,27 +188,28 @@ static int get_fpregs(struct user_i387_struct __user *buf, struct task_struct *c
 
 static int set_fpregs(struct user_i387_struct __user *buf, struct task_struct *child)
 {
-	int n, cpu = task_cpu(child);
-	struct user_i387_struct fpregs;
+	int n;
+
+	if (have_fpx_regs)
+		return -EINVAL;
 
-	n = copy_from_user(&fpregs, buf, sizeof(fpregs));
+	n = copy_from_user(&child->thread.regs.regs.fp, buf,
+			   sizeof(struct user_i387_struct));
 	if (n > 0)
 		return -EFAULT;
 
-	return restore_i387_registers(userspace_pid[cpu],
-				    (unsigned long *) &fpregs);
+	return 0;
 }
 
 static int get_fpxregs(struct user_fxsr_struct __user *buf, struct task_struct *child)
 {
-	int err, n, cpu = task_cpu(child);
-	struct user_fxsr_struct fpregs;
+	int n;
 
-	err = save_fpx_registers(userspace_pid[cpu], (unsigned long *) &fpregs);
-	if (err)
-		return err;
+	if (!have_fpx_regs)
+		return -EINVAL;
 
-	n = copy_to_user(buf, &fpregs, sizeof(fpregs));
+	n = copy_to_user(buf, &child->thread.regs.regs.fp,
+			 sizeof(struct user_fxsr_struct));
 	if(n > 0)
 		return -EFAULT;
 
@@ -216,15 +218,17 @@ static int get_fpxregs(struct user_fxsr_struct __user *buf, struct task_struct *
 
 static int set_fpxregs(struct user_fxsr_struct __user *buf, struct task_struct *child)
 {
-	int n, cpu = task_cpu(child);
-	struct user_fxsr_struct fpregs;
+	int n;
 
-	n = copy_from_user(&fpregs, buf, sizeof(fpregs));
+	if (!have_fpx_regs)
+		return -EINVAL;
+
+	n = copy_from_user(&child->thread.regs.regs.fp, buf,
+			   sizeof(struct user_fxsr_struct));
 	if (n > 0)
 		return -EFAULT;
 
-	return restore_fpx_registers(userspace_pid[cpu],
-				     (unsigned long *) &fpregs);
+	return 0;
 }
 
 long subarch_ptrace(struct task_struct *child, long request,
diff --git a/arch/x86/um/ptrace_64.c b/arch/x86/um/ptrace_64.c
index aa68d83d3f44..f8bbad29cd0b 100644
--- a/arch/x86/um/ptrace_64.c
+++ b/arch/x86/um/ptrace_64.c
@@ -190,15 +190,10 @@ int peek_user(struct task_struct *child, long addr, long data)
 
 static int get_fpregs(struct user_i387_struct __user *buf, struct task_struct *child)
 {
-	int err, n, cpu = ((struct thread_info *) child->stack)->cpu;
-	struct user_i387_struct fpregs;
+	int n;
 
-	err = save_i387_registers(userspace_pid[cpu],
-				  (unsigned long *) &fpregs);
-	if (err)
-		return err;
-
-	n = copy_to_user(buf, &fpregs, sizeof(fpregs));
+	n = copy_to_user(buf, &child->thread.regs.regs.fp,
+			 sizeof(struct user_i387_struct));
 	if (n > 0)
 		return -EFAULT;
 
@@ -207,15 +202,14 @@ static int get_fpregs(struct user_i387_struct __user *buf, struct task_struct *c
 
 static int set_fpregs(struct user_i387_struct __user *buf, struct task_struct *child)
 {
-	int n, cpu = ((struct thread_info *) child->stack)->cpu;
-	struct user_i387_struct fpregs;
+	int n;
 
-	n = copy_from_user(&fpregs, buf, sizeof(fpregs));
+	n = copy_from_user(&child->thread.regs.regs.fp, buf,
+			   sizeof(struct user_i387_struct));
 	if (n > 0)
 		return -EFAULT;
 
-	return restore_i387_registers(userspace_pid[cpu],
-				      (unsigned long *) &fpregs);
+	return 0;
 }
 
 long subarch_ptrace(struct task_struct *child, long request,
diff --git a/arch/x86/um/signal.c b/arch/x86/um/signal.c
index 2cc8c2309022..fa78c74e00cb 100644
--- a/arch/x86/um/signal.c
+++ b/arch/x86/um/signal.c
@@ -204,34 +204,30 @@ static int copy_sc_from_user(struct pt_regs *regs,
 
 #ifdef CONFIG_X86_32
 	if (have_fpx_regs) {
-		struct user_fxsr_struct fpx;
-		int pid = userspace_pid[current_thread_info()->cpu];
+		struct user_fxsr_struct *fpx;
+		fpx = (void *)&regs->regs.fp;
 
-		err = copy_from_user(&fpx,
-			&((struct _fpstate __user *)sc.fpstate)->_fxsr_env[0],
-				     sizeof(struct user_fxsr_struct));
+		err = convert_fxsr_from_user(fpx, (void *)sc.fpstate);
 		if (err)
 			return 1;
-
-		err = convert_fxsr_from_user(&fpx, (void *)sc.fpstate);
+	} else {
+		BUILD_BUG_ON(sizeof(regs->regs.fp) > sizeof(struct _fpstate));
+		err = copy_from_user(regs->regs.fp, (void *)sc.fpstate,
+				     sizeof(regs->regs.fp));
 		if (err)
 			return 1;
-
-		err = restore_fpx_registers(pid, (unsigned long *) &fpx);
-		if (err < 0) {
-			printk(KERN_ERR "copy_sc_from_user - "
-			       "restore_fpx_registers failed, errno = %d\n",
-			       -err);
-			return 1;
-		}
-	} else
-#endif
+	}
+#else
 	{
+		/* FIXME: Save/restore extended state (past the 16 YMM regs) */
+		BUILD_BUG_ON(sizeof(regs->regs.fp) < sizeof(struct _xstate));
+
 		err = copy_from_user(regs->regs.fp, (void *)sc.fpstate,
 				     sizeof(struct _xstate));
 		if (err)
 			return 1;
 	}
+#endif
 	return 0;
 }
 
@@ -291,34 +287,35 @@ static int copy_sc_to_user(struct sigcontext __user *to,
 
 #ifdef CONFIG_X86_32
 	if (have_fpx_regs) {
-		int pid = userspace_pid[current_thread_info()->cpu];
-		struct user_fxsr_struct fpx;
+		struct user_fxsr_struct *fpx;
 
-		err = save_fpx_registers(pid, (unsigned long *) &fpx);
-		if (err < 0){
-			printk(KERN_ERR "copy_sc_to_user - save_fpx_registers "
-			       "failed, errno = %d\n", err);
-			return 1;
-		}
+		fpx = (void *)&regs->regs.fp;
 
-		err = convert_fxsr_to_user(&to_fp->fpstate, &fpx);
+		err = convert_fxsr_to_user(&to_fp->fpstate, fpx);
 		if (err)
 			return 1;
 
-		err |= __put_user(fpx.swd, &to_fp->fpstate.status);
+		err |= __put_user(fpx->swd, &to_fp->fpstate.status);
 		err |= __put_user(X86_FXSR_MAGIC, &to_fp->fpstate.magic);
 		if (err)
 			return 1;
 
-		if (copy_to_user(&to_fp->fpstate._fxsr_env[0], &fpx,
-				 sizeof(struct user_fxsr_struct)))
-			return 1;
-	} else
-#endif
-	{
-		if (copy_to_user(to_fp, regs->regs.fp, sizeof(struct _xstate)))
+	} else {
+		if (copy_to_user(to_fp, regs->regs.fp, sizeof(regs->regs.fp)))
 			return 1;
+
+		/* Need to fill in the sw_reserved bits ... */
+		BUILD_BUG_ON(offsetof(typeof(*to_fp),
+				      fpstate.sw_reserved.magic1) >=
+			     sizeof(struct _fpstate));
+		__put_user(0, &to_fp->fpstate.sw_reserved.magic1);
+		__put_user(sizeof(struct _fpstate),
+			   &to_fp->fpstate.sw_reserved.extended_size);
 	}
+#else
+	if (copy_to_user(to_fp, regs->regs.fp, sizeof(struct _xstate)))
+		return 1;
+#endif
 
 	return 0;
 }
author	Benjamin Berg <benjamin.berg@intel.com>	2024-09-13 15:38:45 +0200
committer	Johannes Berg <johannes.berg@intel.com>	2024-10-10 12:03:55 +0200
commit	5a6951273e0e9dc0f79facf22281a8a731fb90b1 (patch)
tree	e8b65704dc743bcfd211950bd7015f7972b745a4
parent	865e3845eeaa21e9a62abc1361644e67124f1ec0 (diff)
download	lwn-5a6951273e0e9dc0f79facf22281a8a731fb90b1.tar.gz lwn-5a6951273e0e9dc0f79facf22281a8a731fb90b1.zip