Hi Everyone, I'm following up on the ptrace() problem that I reported a few days ago. I believe my version of the code handles all cases correctly. While the problem essentially boils down to dividing the fpidx by 2 on PPC32, it becomes tricky when the same code must work correctly on both PPC32 and PPC64.
One other thing that I believe was handled incorrectly in the previous version is the unused half of fpscr on PPC32. Note that while PT_FPSCR is defined as (PT_FPR0 + 2*32 + 1), making only the upper half visible, PT_FPR0 + 2*32 still corresponds to a possible address that userspace can pass. In that case, comparing fpidx to (PT_FPSCR - PT_FPR0) would cause an invalid access to the FPU registers array. I tested the patch on 4.9.179, but that part of the code is identical in recent kernels so it should work just the same. I wrote a simple test program than can be used to quickly test (on an x86_64 host) that all cases are handled correctly for both PPC32/PPC64. The code is included below. I also tested with gdbserver (test patch included below) and verified that it generates two ptrace() calls for each FPU register, with addresses between 0xc0 and 0x1bc. 8<--------------- Makefile --------------------------------------------- .PHONY: all clean all: ptrace-fpregs-32 ptrace-fpregs-64 ptrace-fpregs-32: ptrace-fpregs.c $(CC) -o ptrace-fpregs-32 -Wall -O2 -m32 ptrace-fpregs.c ptrace-fpregs-64: ptrace-fpregs.c $(CC) -o ptrace-fpregs-64 -Wall -O2 ptrace-fpregs.c clean: rm -f ptrace-fpregs-32 ptrace-fpregs-64 8<--------------- ptrace-fpregs.c -------------------------------------- #include <stdio.h> #include <errno.h> #define PT_FPR0 48 #ifndef __x86_64 #define PT_FPR31 (PT_FPR0 + 2*31) #define PT_FPSCR (PT_FPR0 + 2*32 + 1) #else #define PT_FPSCR (PT_FPR0 + 32) #endif int test_access(unsigned long addr) { int ret; do { unsigned long index, fpidx; ret = -EIO; /* convert to index and check */ index = addr / sizeof(long); if ((addr & (sizeof(long) - 1)) || (index > PT_FPSCR)) break; if (index < PT_FPR0) { ret = printf("ptrace_put_reg(%lu)", index); break; } ret = 0; #ifndef __x86_64 if (index == PT_FPSCR - 1) { /* corner case for PPC32; do nothing */ printf("corner_case"); break; } #endif if (index == PT_FPSCR) { printf("fpscr"); 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; printf("TS_FPR[%lu] + %lu", fpidx, addr & 4); break; } while (0); return ret; } int main(void) { unsigned long addr; int rc; for (addr = 0; addr < PT_FPSCR * sizeof(long) + 16; addr++) { printf("0x%04lx: ", addr); rc = test_access(addr); if (rc < 0) printf("!err!"); printf("\t<%d>\n", rc); } return 0; } 8<--------------- gdb.patch -------------------------------------------- --- gdb/gdbserver/linux-low.c.orig 2019-06-10 11:45:53.810882669 -0400 +++ gdb/gdbserver/linux-low.c 2019-06-10 11:49:32.272929766 -0400 @@ -4262,6 +4262,8 @@ store_register (struct regcache *regcach pid = lwpid_of (get_thread_lwp (current_inferior)); for (i = 0; i < size; i += sizeof (PTRACE_XFER_TYPE)) { + printf("writing register #%d offset %d at address %#x\n", + regno, i, (unsigned int)regaddr); errno = 0; ptrace (PTRACE_POKEUSER, pid, /* Coerce to a uintptr_t first to avoid potential gcc warning 8<---------------------------------------------------------------------- Radu Rendec (1): PPC32: fix ptrace() access to FPU registers arch/powerpc/kernel/ptrace.c | 85 ++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 33 deletions(-) -- 2.20.1