| From be07a6a1188372b6d19a3307ec33211fc9c9439d Mon Sep 17 00:00:00 2001 |
| From: "Maciej W. Rozycki" <macro@mips.com> |
| Date: Mon, 11 Dec 2017 22:54:33 +0000 |
| Subject: MIPS: Fix an FCSR access API regression with NT_PRFPREG and MSA |
| |
| From: Maciej W. Rozycki <macro@mips.com> |
| |
| commit be07a6a1188372b6d19a3307ec33211fc9c9439d upstream. |
| |
| Fix a commit 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for |
| FP regset") public API regression, then activated by commit 1db1af84d6df |
| ("MIPS: Basic MSA context switching support"), that caused the FCSR |
| register not to be read or written for CONFIG_CPU_HAS_MSA kernel |
| configurations (regardless of actual presence or absence of the MSA |
| feature in a given processor) with ptrace(2) PTRACE_GETREGSET and |
| PTRACE_SETREGSET requests nor recorded in core dumps. |
| |
| This is because with !CONFIG_CPU_HAS_MSA configurations the whole of |
| `elf_fpregset_t' array is bulk-copied as it is, which includes the FCSR |
| in one half of the last, 33rd slot, whereas with CONFIG_CPU_HAS_MSA |
| configurations array elements are copied individually, and then only the |
| leading 32 FGR slots while the remaining slot is ignored. |
| |
| Correct the code then such that only FGR slots are copied in the |
| respective !MSA and MSA helpers an then the FCSR slot is handled |
| separately in common code. Use `ptrace_setfcr31' to update the FCSR |
| too, so that the read-only mask is respected. |
| |
| Retrieving a correct value of FCSR is important in debugging not only |
| for the human to be able to get the right interpretation of the |
| situation, but for correct operation of GDB as well. This is because |
| the condition code bits in FSCR are used by GDB to determine the |
| location to place a breakpoint at when single-stepping through an FPU |
| branch instruction. If such a breakpoint is placed incorrectly (i.e. |
| with the condition reversed), then it will be missed, likely causing the |
| debuggee to run away from the control of GDB and consequently breaking |
| the process of investigation. |
| |
| Fortunately GDB continues using the older PTRACE_GETFPREGS ptrace(2) |
| request which is unaffected, so the regression only really hits with |
| post-mortem debug sessions using a core dump file, in which case |
| execution, and consequently single-stepping through branches is not |
| possible. Of course core files created by buggy kernels out there will |
| have the value of FCSR recorded clobbered, but such core files cannot be |
| corrected and the person using them simply will have to be aware that |
| the value of FCSR retrieved is not reliable. |
| |
| Which also means we can likely get away without defining a replacement |
| API which would ensure a correct value of FSCR to be retrieved, or none |
| at all. |
| |
| This is based on previous work by Alex Smith, extensively rewritten. |
| |
| Signed-off-by: Alex Smith <alex@alex-smith.me.uk> |
| Signed-off-by: James Hogan <james.hogan@mips.com> |
| Signed-off-by: Maciej W. Rozycki <macro@mips.com> |
| Fixes: 72b22bbad1e7 ("MIPS: Don't assume 64-bit FP registers for FP regset") |
| Cc: Paul Burton <Paul.Burton@mips.com> |
| Cc: Dave Martin <Dave.Martin@arm.com> |
| Cc: linux-mips@linux-mips.org |
| Cc: linux-kernel@vger.kernel.org |
| Patchwork: https://patchwork.linux-mips.org/patch/17928/ |
| Signed-off-by: Ralf Baechle <ralf@linux-mips.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| arch/mips/kernel/ptrace.c | 47 +++++++++++++++++++++++++++++++++++----------- |
| 1 file changed, 36 insertions(+), 11 deletions(-) |
| |
| --- a/arch/mips/kernel/ptrace.c |
| +++ b/arch/mips/kernel/ptrace.c |
| @@ -413,7 +413,7 @@ static int gpr64_set(struct task_struct |
| /* |
| * Copy the floating-point context to the supplied NT_PRFPREG buffer, |
| * !CONFIG_CPU_HAS_MSA variant. FP context's general register slots |
| - * correspond 1:1 to buffer slots. |
| + * correspond 1:1 to buffer slots. Only general registers are copied. |
| */ |
| static int fpr_get_fpa(struct task_struct *target, |
| unsigned int *pos, unsigned int *count, |
| @@ -421,13 +421,14 @@ static int fpr_get_fpa(struct task_struc |
| { |
| return user_regset_copyout(pos, count, kbuf, ubuf, |
| &target->thread.fpu, |
| - 0, sizeof(elf_fpregset_t)); |
| + 0, NUM_FPU_REGS * sizeof(elf_fpreg_t)); |
| } |
| |
| /* |
| * Copy the floating-point context to the supplied NT_PRFPREG buffer, |
| * CONFIG_CPU_HAS_MSA variant. Only lower 64 bits of FP context's |
| - * general register slots are copied to buffer slots. |
| + * general register slots are copied to buffer slots. Only general |
| + * registers are copied. |
| */ |
| static int fpr_get_msa(struct task_struct *target, |
| unsigned int *pos, unsigned int *count, |
| @@ -449,20 +450,29 @@ static int fpr_get_msa(struct task_struc |
| return 0; |
| } |
| |
| -/* Copy the floating-point context to the supplied NT_PRFPREG buffer. */ |
| +/* |
| + * Copy the floating-point context to the supplied NT_PRFPREG buffer. |
| + * Choose the appropriate helper for general registers, and then copy |
| + * the FCSR register separately. |
| + */ |
| static int fpr_get(struct task_struct *target, |
| const struct user_regset *regset, |
| unsigned int pos, unsigned int count, |
| void *kbuf, void __user *ubuf) |
| { |
| + const int fcr31_pos = NUM_FPU_REGS * sizeof(elf_fpreg_t); |
| int err; |
| |
| - /* XXX fcr31 */ |
| - |
| if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t)) |
| err = fpr_get_fpa(target, &pos, &count, &kbuf, &ubuf); |
| else |
| err = fpr_get_msa(target, &pos, &count, &kbuf, &ubuf); |
| + if (err) |
| + return err; |
| + |
| + err = user_regset_copyout(&pos, &count, &kbuf, &ubuf, |
| + &target->thread.fpu.fcr31, |
| + fcr31_pos, fcr31_pos + sizeof(u32)); |
| |
| return err; |
| } |
| @@ -470,7 +480,7 @@ static int fpr_get(struct task_struct *t |
| /* |
| * Copy the supplied NT_PRFPREG buffer to the floating-point context, |
| * !CONFIG_CPU_HAS_MSA variant. Buffer slots correspond 1:1 to FP |
| - * context's general register slots. |
| + * context's general register slots. Only general registers are copied. |
| */ |
| static int fpr_set_fpa(struct task_struct *target, |
| unsigned int *pos, unsigned int *count, |
| @@ -478,13 +488,14 @@ static int fpr_set_fpa(struct task_struc |
| { |
| return user_regset_copyin(pos, count, kbuf, ubuf, |
| &target->thread.fpu, |
| - 0, sizeof(elf_fpregset_t)); |
| + 0, NUM_FPU_REGS * sizeof(elf_fpreg_t)); |
| } |
| |
| /* |
| * Copy the supplied NT_PRFPREG buffer to the floating-point context, |
| * CONFIG_CPU_HAS_MSA variant. Buffer slots are copied to lower 64 |
| - * bits only of FP context's general register slots. |
| + * bits only of FP context's general register slots. Only general |
| + * registers are copied. |
| */ |
| static int fpr_set_msa(struct task_struct *target, |
| unsigned int *pos, unsigned int *count, |
| @@ -509,6 +520,8 @@ static int fpr_set_msa(struct task_struc |
| |
| /* |
| * Copy the supplied NT_PRFPREG buffer to the floating-point context. |
| + * Choose the appropriate helper for general registers, and then copy |
| + * the FCSR register separately. |
| * |
| * We optimize for the case where `count % sizeof(elf_fpreg_t) == 0', |
| * which is supposed to have been guaranteed by the kernel before |
| @@ -521,18 +534,30 @@ static int fpr_set(struct task_struct *t |
| unsigned int pos, unsigned int count, |
| const void *kbuf, const void __user *ubuf) |
| { |
| + const int fcr31_pos = NUM_FPU_REGS * sizeof(elf_fpreg_t); |
| + u32 fcr31; |
| int err; |
| |
| BUG_ON(count % sizeof(elf_fpreg_t)); |
| |
| - /* XXX fcr31 */ |
| - |
| init_fp_ctx(target); |
| |
| if (sizeof(target->thread.fpu.fpr[0]) == sizeof(elf_fpreg_t)) |
| err = fpr_set_fpa(target, &pos, &count, &kbuf, &ubuf); |
| else |
| err = fpr_set_msa(target, &pos, &count, &kbuf, &ubuf); |
| + if (err) |
| + return err; |
| + |
| + if (count > 0) { |
| + err = user_regset_copyin(&pos, &count, &kbuf, &ubuf, |
| + &fcr31, |
| + fcr31_pos, fcr31_pos + sizeof(u32)); |
| + if (err) |
| + return err; |
| + |
| + ptrace_setfcr31(target, fcr31); |
| + } |
| |
| return err; |
| } |