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