[PATCH] Fix ptrace compat wrapper for fpu register access
Michael Neuling
mikey at neuling.org
Mon Apr 6 16:59:39 EST 2009
In message <8394.1239000531 at neuling.org> you wrote:
> > > >> @@ -263,7 +263,9 @@ long compat_arch_ptrace(struct task_stru
> > > >> ret = ptrace_put_reg(child, numReg, freg);
> > > >> } else {
> > > >> flush_fp_to_thread(child);
> > > >> - ((unsigned int *)child->thread.regs)[index] = d
> ata;
> > > >> + ((unsigned int *)child->thread.fpr)
> > > >> + [TS_FPRWIDTH * (numReg - PT_FPR0) * 2 +
> > > >> + index % 2] = data;
> > > >
> > > > I think the indexing here should be the same as PEEKUSR_3264. This
> > > > looks better but all this magic indexing makes me want to vomit.
> > >
> > > How about this instead:
> > >
> > > @@ -241,6 +241,7 @@ long compat_arch_ptrace(struct task_stru
> > > case PPC_PTRACE_POKEUSR_3264: {
> > > u32 index;
> > > u32 numReg;
> > > + u32 *tmp;
> > >
> > > ret = -EIO;
> > > /* Determine which register the user wants */
> > > @@ -263,7 +264,8 @@ long compat_arch_ptrace(struct task_stru
> > > ret = ptrace_put_reg(child, numReg, freg);
> > > } else {
> > > flush_fp_to_thread(child);
> > > - ((unsigned int *)child->thread.regs)[index] = data;
> > > + tmp = (u32 *)child->thread.fpr[numReg - PT_FPR0];
> > > + tmp[index % 2] = data;
> >
> > I do like this approach better (two arrays) but there is no accounting
> > for TS_WIDTH, so I'm not sure it works.
>
> Ok, here's another stab at trying to fix this and clean it up.
... and now without the c++ style comments...
Mikey
powerpc: Fix ptrace compat wrapper for fpu register access
The ptrace compat wrapper mishandles access to the fpu registers. The
PTRACE_PEEKUSR and PTRACE_POKEUSR requests miscalculate the index into
the fpr array due to the broken FPINDEX macro. The
PPC_PTRACE_PEEKUSR_3264 request needs to use the same formula that the
native ptrace interface uses when operating on the register number (as
opposed to the 4-byte offset). The PPC_PTRACE_POKEUSR_3264 request
didn't take TS_FPRWIDTH into account.
Signed-off-by: Andreas Schwab <schwab at linux-m68k.org>
Signed-off-by: Michael Neuling <mikey at neuling.org>
---
arch/powerpc/kernel/ptrace32.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
Index: linux-2.6-ozlabs/arch/powerpc/kernel/ptrace32.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/ptrace32.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/ptrace32.c
@@ -67,7 +67,8 @@ static long compat_ptrace_old(struct tas
/* Macros to workout the correct index for the FPR in the thread struct */
#define FPRNUMBER(i) (((i) - PT_FPR0) >> 1)
#define FPRHALF(i) (((i) - PT_FPR0) & 1)
-#define FPRINDEX(i) TS_FPRWIDTH * FPRNUMBER(i) + FPRHALF(i)
+#define FPRINDEX(i) TS_FPRWIDTH * FPRNUMBER(i) * 2 + FPRHALF(i)
+#define FPRINDEX_3264(i) (TS_FPRWIDTH * ((i) - PT_FPR0))
long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
compat_ulong_t caddr, compat_ulong_t cdata)
@@ -168,8 +169,9 @@ long compat_arch_ptrace(struct task_stru
CHECK_FULL_REGS(child->thread.regs);
if (numReg >= PT_FPR0) {
flush_fp_to_thread(child);
- tmp = ((unsigned long int *)child->thread.fpr)
- [FPRINDEX(numReg)];
+ /* get 64 bit FPR */
+ tmp = ((u64 *)child->thread.fpr)
+ [FPRINDEX_3264(numReg)];
} else { /* register within PT_REGS struct */
tmp = ptrace_get_reg(child, numReg);
}
@@ -262,8 +264,13 @@ long compat_arch_ptrace(struct task_stru
freg = (freg & 0xfffffffful) | (data << 32);
ret = ptrace_put_reg(child, numReg, freg);
} else {
+ u64 *tmp;
flush_fp_to_thread(child);
- ((unsigned int *)child->thread.regs)[index] = data;
+ /* get 64 bit FPR ... */
+ tmp = &(((u64 *)child->thread.fpr)
+ [FPRINDEX_3264(numReg)]);
+ /* ... write the 32 bit part we want */
+ ((u32 *)tmp)[index % 2] = data;
ret = 0;
}
break;
More information about the Linuxppc-dev
mailing list