[PATCH 0/1] PPC32: fix ptrace() access to FPU registers

[Radu Rendec]

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