Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

On Wed, 2006-10-25 at 16:23 +0530, supriya kannery wrote:
> In ptrace, when request is PPC_PTRACE_GETREGS, SETREGS, GETFPREGS and 
> SETFPREGS, order of the last two arguments is not correct.
> 
> General format of ptrace is ptrace (request, pid, addr, data).  For the 
> above mentioned request ids in ppc64, if we use ptrace like
> 
>  long reg[32];
>  ptrace (PPC_PTRACE_GETREGS, pid, 0, &reg[0]);
> 
> the return value is always -1.
> 
> If we exchange the last two arguments like,
> 
>  ptrace (PPC_PTRACE_GETREGS, pid, &reg[0], 0);
> 
> it works!
> 
> This is because PPC_PTRACE_GETREGS option for powerpc is implemented 
> such that general purpose
> registers of the child process get copied to the address variable 
> instead of data variable. Same is
> the case with other PPC request options PPC_PTRACE_SETREGS, GETFPREGS 
> and SETFPREGS.
> 
> Prepared a patch for this problem and tested with 2.6.18-rc6 kernel. 
> This patch can be applied directly to 2.6.19-rc3 kernel.

A more appropriate place to send this would be the linux-ppc development
list.

-- 
dwmw2

[-- Attachment #2: ppc_ptrace_params.patch --]
[-- Type: text/x-patch, Size: 3448 bytes --]

diff -Naurp linux-2.6.18-rc6/arch/powerpc/kernel/ptrace.c linux-2.6.18-rc6-mod/arch/powerpc/kernel/ptrace.c
--- linux-2.6.18-rc6/arch/powerpc/kernel/ptrace.c	2006-10-19 18:09:01.000000000 +0530
+++ linux-2.6.18-rc6-mod/arch/powerpc/kernel/ptrace.c	2006-10-19 18:11:37.000000000 +0530
@@ -406,7 +406,7 @@ long arch_ptrace(struct task_struct *chi
 	case PPC_PTRACE_GETREGS: { /* Get GPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
-		unsigned long __user *tmp = (unsigned long __user *)addr;
+		unsigned long __user *tmp = (unsigned long __user *)data;
 
 		for (i = 0; i < 32; i++) {
 			ret = put_user(*reg, tmp);
@@ -421,7 +421,7 @@ long arch_ptrace(struct task_struct *chi
 	case PPC_PTRACE_SETREGS: { /* Set GPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
-		unsigned long __user *tmp = (unsigned long __user *)addr;
+		unsigned long __user *tmp = (unsigned long __user *)data;
 
 		for (i = 0; i < 32; i++) {
 			ret = get_user(*reg, tmp);
@@ -436,7 +436,7 @@ long arch_ptrace(struct task_struct *chi
 	case PPC_PTRACE_GETFPREGS: { /* Get FPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.fpr)[0];
-		unsigned long __user *tmp = (unsigned long __user *)addr;
+		unsigned long __user *tmp = (unsigned long __user *)data;
 
 		flush_fp_to_thread(child);
 
@@ -453,7 +453,7 @@ long arch_ptrace(struct task_struct *chi
 	case PPC_PTRACE_SETFPREGS: { /* Get FPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.fpr)[0];
-		unsigned long __user *tmp = (unsigned long __user *)addr;
+		unsigned long __user *tmp = (unsigned long __user *)data;
 
 		flush_fp_to_thread(child);
 
diff -Naurp linux-2.6.18-rc6/arch/powerpc/kernel/ptrace32.c linux-2.6.18-rc6-mod/arch/powerpc/kernel/ptrace32.c
--- linux-2.6.18-rc6/arch/powerpc/kernel/ptrace32.c	2006-10-19 18:09:01.000000000 +0530
+++ linux-2.6.18-rc6-mod/arch/powerpc/kernel/ptrace32.c	2006-10-19 18:11:41.000000000 +0530
@@ -345,7 +345,7 @@ long compat_sys_ptrace(int request, int 
 	case PPC_PTRACE_GETREGS: { /* Get GPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
-		unsigned int __user *tmp = (unsigned int __user *)addr;
+		unsigned int __user *tmp = (unsigned int __user *)data;
 
 		for (i = 0; i < 32; i++) {
 			ret = put_user(*reg, tmp);
@@ -360,7 +360,7 @@ long compat_sys_ptrace(int request, int 
 	case PPC_PTRACE_SETREGS: { /* Set GPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
-		unsigned int __user *tmp = (unsigned int __user *)addr;
+		unsigned int __user *tmp = (unsigned int __user *)data;
 
 		for (i = 0; i < 32; i++) {
 			ret = get_user(*reg, tmp);
@@ -375,7 +375,7 @@ long compat_sys_ptrace(int request, int 
 	case PPC_PTRACE_GETFPREGS: { /* Get FPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.fpr)[0];
-		unsigned int __user *tmp = (unsigned int __user *)addr;
+		unsigned int __user *tmp = (unsigned int __user *)data;
 
 		flush_fp_to_thread(child);
 
@@ -392,7 +392,7 @@ long compat_sys_ptrace(int request, int 
 	case PPC_PTRACE_SETFPREGS: { /* Get FPRs 0 - 31. */
 		int i;
 		unsigned long *reg = &((unsigned long *)child->thread.fpr)[0];
-		unsigned int __user *tmp = (unsigned int __user *)addr;
+		unsigned int __user *tmp = (unsigned int __user *)data;
 
 		flush_fp_to_thread(child);
 

Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

[Benjamin Herrenschmidt]

On Wed, 2006-11-22 at 18:36 +0000, David Woodhouse wrote:
> On
> > This is because PPC_PTRACE_GETREGS option for powerpc is implemented 
> > such that general purpose
> > registers of the child process get copied to the address variable 
> > instead of data variable. Same is
> > the case with other PPC request options PPC_PTRACE_SETREGS, GETFPREGS 
> > and SETFPREGS.
> > 
> > Prepared a patch for this problem and tested with 2.6.18-rc6 kernel. 
> > This patch can be applied directly to 2.6.19-rc3 kernel.
> 
> A more appropriate place to send this would be the linux-ppc development
> list.

Also it's possible that existing code like gcc relies on that "feature"
no ?

Ben.
 

Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

[supriya kannery]

Benjamin Herrenschmidt wrote:

>On Wed, 2006-11-22 at 18:36 +0000, David Woodhouse wrote:
>  
>
>>On
>>    
>>
>>>This is because PPC_PTRACE_GETREGS option for powerpc is implemented
>>>such that general purpose
>>>registers of the child process get copied to the address variable
>>>instead of data variable. Same is
>>>the case with other PPC request options PPC_PTRACE_SETREGS, GETFPREGS
>>>and SETFPREGS.
>>>
>>>Prepared a patch for this problem and tested with 2.6.18-rc6 kernel.
>>>This patch can be applied directly to 2.6.19-rc3 kernel.
>>>      
>>>
>>A more appropriate place to send this would be the linux-ppc development
>>list.
>>    
>>
>
>Also it's possible that existing code like gcc relies on that "feature"
>no ?
>
>Ben.
>
>
>  
>
Thanks David for posting this in ppc-dev list.
    I checked in GDB and ltrace which uses ptrace(). None of them are 
using PPC_PTRACE* options to get register values. The reasons for its 
less (or no) usage could be
1. These options are not documented in manpage
2. The usage is different from the general format of ptrace
3. These are used for copying all the registers. Most applications will 
require data from a single addr/register at a time and can get this data 
from a specific register/address using PEEKTEXT or POKETEXT options.
Supriya

Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

[Anton Blanchard]

Hi,

> > In ptrace, when request is PPC_PTRACE_GETREGS, SETREGS, GETFPREGS and 
> > SETFPREGS, order of the last two arguments is not correct.
> > 
> > General format of ptrace is ptrace (request, pid, addr, data).  For the 
> > above mentioned request ids in ppc64, if we use ptrace like
> > 
> >  long reg[32];
> >  ptrace (PPC_PTRACE_GETREGS, pid, 0, &reg[0]);
> > 
> > the return value is always -1.
> > 
> > If we exchange the last two arguments like,
> > 
> >  ptrace (PPC_PTRACE_GETREGS, pid, &reg[0], 0);
> > 
> > it works!
> > 
> > This is because PPC_PTRACE_GETREGS option for powerpc is implemented 
> > such that general purpose
> > registers of the child process get copied to the address variable 
> > instead of data variable. Same is
> > the case with other PPC request options PPC_PTRACE_SETREGS, GETFPREGS 
> > and SETFPREGS.
> > 
> > Prepared a patch for this problem and tested with 2.6.18-rc6 kernel. 
> > This patch can be applied directly to 2.6.19-rc3 kernel.

I looked at this a while ago and my decision at the time was to keep the
old implementation around for a while and create two new ones that match
the x86 numbering:

#define PTRACE_GETREGS            12
#define PTRACE_SETREGS            13
#define PTRACE_GETFPREGS          14
#define PTRACE_SETFPREGS          15

I hate gratuitous differences, each ptrace app ends up with a sea of
ifdefs.

Also I think it would be worth changing getregs/setregs to grab the
entire pt_regs structure. Otherwise most ops (gdb, strace etc) will just
have to make multiple ptrace calls to get the nia etc.

Anton

Re: Incorrect order of last two arguments of ptrace for requests PPC_PTRACE_GETREGS, SETREGS, GETFPREGS, SETFPREGS

[Paul Mackerras]

Anton Blanchard writes:

> I looked at this a while ago and my decision at the time was to keep the
> old implementation around for a while and create two new ones that match
> the x86 numbering:
> 
> #define PTRACE_GETREGS            12
> #define PTRACE_SETREGS            13
> #define PTRACE_GETFPREGS          14
> #define PTRACE_SETFPREGS          15
> 
> I hate gratuitous differences, each ptrace app ends up with a sea of
> ifdefs.
> 
> Also I think it would be worth changing getregs/setregs to grab the
> entire pt_regs structure. Otherwise most ops (gdb, strace etc) will just
> have to make multiple ptrace calls to get the nia etc.

Did you do a patch to do that?

Paul.