This patch addresses several issues with ptrace() access to FPU
registers through PTRACE_PEEKUSR/PTRACE_POKEUSR.

Standard CPU registers are of course the size of the machine word on
both PPC32/PPC64, but FPU registers are always 64-bit. Because the
ptrace() can only transfer one `long` at a time with PTRACE_PEEKUSR and
PTRACE_POKEUSR, on PPC32 userspace must do two separate ptrace() calls
to access a whole FPU register.

This patch fixes the code that translates between ptrace() addresses and
indexes into (struct thread_fp_state).fpr, taking into account all cases
for both PPC32/PPC64. In the previous version, on PPC32, the index was
double the correct value, allowing memory to be accessed beyond the
register array. This had the following side effects:
* Access to all FPU registers (except for FPR0) was broken.
* PTRACE_POKEUSR could corrupt memory following the FPU register array.
  That means the remainder of thread_struct, which is by design the last
  field of task_struct. For large enough ptrace() addresses, memory
  access could go even outside task_struct, corrupting the adjacent
  task_struct.

Note that gdb (which is probably the most frequent user of ptrace() with
PTRACE_PEEKUSR/PTRACE_POKEUSR) seems to always read/write all FPU
registers whenever a traced task stops.

Signed-off-by: Radu Rendec <radu.ren...@gmail.com>
---
 arch/powerpc/kernel/ptrace.c | 85 ++++++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 684b0b315c32..060e5ed0fad9 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -2991,69 +2991,88 @@ long arch_ptrace(struct task_struct *child, long 
request,
        switch (request) {
        /* read the word at location addr in the USER area. */
        case PTRACE_PEEKUSR: {
-               unsigned long index, tmp;
+               unsigned long index, fpidx, tmp = 0;
 
                ret = -EIO;
                /* convert to index and check */
+               index = addr / sizeof(long);
+               if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR))
+                       break;
 #ifdef CONFIG_PPC32
-               index = addr >> 2;
-               if ((addr & 3) || (index > PT_FPSCR)
-                   || (child->thread.regs == NULL))
-#else
-               index = addr >> 3;
-               if ((addr & 7) || (index > PT_FPSCR))
-#endif
+               if (child->thread.regs == NULL)
                        break;
+#endif
 
                CHECK_FULL_REGS(child->thread.regs);
                if (index < PT_FPR0) {
                        ret = ptrace_get_reg(child, (int) index, &tmp);
                        if (ret)
                                break;
-               } else {
-                       unsigned int fpidx = index - PT_FPR0;
+                       goto out_peekusr;
+               }
 
-                       flush_fp_to_thread(child);
-                       if (fpidx < (PT_FPSCR - PT_FPR0))
-                               memcpy(&tmp, &child->thread.TS_FPR(fpidx),
-                                      sizeof(long));
-                       else
-                               tmp = child->thread.fp_state.fpscr;
+               flush_fp_to_thread(child);
+#ifdef CONFIG_PPC32
+               if (index == PT_FPSCR - 1)
+                       /* corner case for PPC32; do nothing */
+                       goto out_peekusr;
+#endif
+               if (index == PT_FPSCR) {
+                       tmp = child->thread.fp_state.fpscr;
+                       goto out_peekusr;
                }
+               /*
+                * FPR is always 64-bit; on PPC32, userspace does two 32-bit
+                * accesses. Add bit2 to allow accessing the upper half on
+                * 32-bit; on 64-bit, bit2 is always 0 (we validate it above).
+                */
+               fpidx = (addr - PT_FPR0 * sizeof(long)) / 8;
+               memcpy(&tmp, (void *)&child->thread.TS_FPR(fpidx) + (addr & 4),
+                       sizeof(long));
+out_peekusr:
                ret = put_user(tmp, datalp);
                break;
        }
 
        /* write the word at location addr in the USER area */
        case PTRACE_POKEUSR: {
-               unsigned long index;
+               unsigned long index, fpidx;
 
                ret = -EIO;
                /* convert to index and check */
+               index = addr / sizeof(long);
+               if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR))
+                       break;
 #ifdef CONFIG_PPC32
-               index = addr >> 2;
-               if ((addr & 3) || (index > PT_FPSCR)
-                   || (child->thread.regs == NULL))
-#else
-               index = addr >> 3;
-               if ((addr & 7) || (index > PT_FPSCR))
-#endif
+               if (child->thread.regs == NULL)
                        break;
+#endif
 
                CHECK_FULL_REGS(child->thread.regs);
                if (index < PT_FPR0) {
                        ret = ptrace_put_reg(child, index, data);
-               } else {
-                       unsigned int fpidx = index - PT_FPR0;
+                       break;
+               }
 
-                       flush_fp_to_thread(child);
-                       if (fpidx < (PT_FPSCR - PT_FPR0))
-                               memcpy(&child->thread.TS_FPR(fpidx), &data,
-                                      sizeof(long));
-                       else
-                               child->thread.fp_state.fpscr = data;
-                       ret = 0;
+               ret = 0;
+               flush_fp_to_thread(child);
+#ifdef CONFIG_PPC32
+               if (index == PT_FPSCR - 1)
+                       /* corner case for PPC32; do nothing */
+                       break;
+#endif
+               if (index == PT_FPSCR) {
+                       child->thread.fp_state.fpscr = data;
+                       break;
                }
+               /*
+                * FPR is always 64-bit; on PPC32, userspace does two 32-bit
+                * accesses. Add bit2 to allow accessing the upper half on
+                * 32-bit; on 64-bit, bit2 is always 0 (we validate it above).
+                */
+               fpidx = (addr - PT_FPR0 * sizeof(long)) / 8;
+               memcpy((void *)&child->thread.TS_FPR(fpidx) + (addr & 4),
+                       &data, sizeof(long));
                break;
        }
 
-- 
2.20.1

Reply via email to