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>

diff --git a/arch/um/include/shared/registers.h b/arch/um/include/shared/registers.h
index a0450326521cd55e0ec81678dbca1ba435d119ee..7d81b2339a48ea4dcc682f1819ff4863e4bfe66c 100644 (file)
--- 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 9c6cf03ed02b03e186a1d64c34ef40cad050d486..701b7bb2525efb51504ffee67a52728f67447abf 100644 (file)
--- 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 f3638dd09cececd365463464f31762a39d0769f6..f8e5516d97084599dde22dd753d0ca210d3ca49a 100644 (file)
--- 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 b0a71c6cdc6e6cdfb1ced4cee1ea58bb0e040663..b8b85a52eb6f97a667b64af002c06729749fc5ea 100644 (file)
--- 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 aa68d83d3f441b746e0b11a9ffa8bec3ca51a996..f8bbad29cd0bd330ee1f6f8de1aa476817946eb8 100644 (file)
--- 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 2cc8c230902287b87711a7bea394779168401f6d..fa78c74e00cbfa8b3177b45e951feacf4c2c7d19 100644 (file)
--- 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;
 }