Bug 211867 - The order of the last two arguments of ptrace is not right when request is PPC_PTRACE_GETREGS, GETFPREGS and SETFPREGS
Summary: The order of the last two arguments of ptrace is not right when request is PP...
Keywords:
Status: CLOSED INSUFFICIENT_DATA
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 7
Hardware: powerpc
OS: Linux
medium
medium
Target Milestone: ---
Assignee: David Woodhouse
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On:
Blocks: 173278 217057
TreeView+ depends on / blocked
 
Reported: 2006-10-23 16:10 UTC by IBM Bug Proxy
Modified: 2008-08-08 07:10 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-04-25 03:48:30 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Test case (4.09 KB, text/plain)
2006-10-23 16:14 UTC, IBM Bug Proxy
no flags Details
Patch against FC6 kernel (3.37 KB, text/plain)
2006-10-23 16:14 UTC, IBM Bug Proxy
no flags Details


Links
System ID Private Priority Status Summary Last Updated
IBM Linux Technology Center 28253 0 None None None Never

Description IBM Bug Proxy 2006-10-23 16:10:38 UTC
LTC Owner is: skannery.com
LTC Originator is: qiyao.com

Problem description:
According to the ptrace man page, the last arugments should be address and data.

if we use ptrace like this,
  long reg[32];
  ptrace (PPC_PTRACE_GETREGS, pid, 0, �[0]);
but the return value is always -1.

If we use ptrace like this,
  ptrace (PPC_PTRACE_GETREGS, pid, �[0], 0);
It works!

It seems that the order last two arguments is reversed!

Provide output from "uname -a", if possible:
[qiyao@plinuxt6 ~/ptrace]$ uname -a
Linux plinuxt6 2.6.17-1.2630.fc6 #1 SMP Wed Sep 6 17:17:26 EDT 2006 ppc64 ppc64
ppc64 GNU/Linux

------------------------------------------------------------------------------------------------
Compile test case in 64-bit mode.

[qiyao@plinuxt6 ~/ptrace]$ gcc -m64 test-ppc_ptrace_getregs.c -o
test-ppc_ptrace_getregs.64
[qiyao@plinuxt6 ~/ptrace]$ ./test-ppc_ptrace_getregs.64 
Child pid: 9933
Start of child.
Child stopped by signal 5
Test PPC_PTRACE_GETREGS:
ERROR: Parent's GETREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's GETREGS succeeded, reversed arg order
Test PPC_PTRACE_GETFPREGS:
ERROR: Parent's GETFPREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's GETFPREGS succeeded, reversed arg order
Test PPC_PTRACE_SETREGS:
ERROR: Parent's SETREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's SETREGS succeeded, reversed arg order
Test PPC_PTRACE_SETFPREGS:
ERROR: Parent's SETFPREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's SETFPREGS succeeded, reversed arg order


We find that ptrace operations(GETREGS, SETREGS, GETFPREGS, SETFPREGS) are
succeeded, but with reversed argument order.

Compile this case in 32-bit mode.
[qiyao@plinuxt6 ~/ptrace]$ gcc -m32 test-ppc_ptrace_getregs.c -o
test-ppc_ptrace_getregs.32
[qiyao@plinuxt6 ~/ptrace]$ ./test-ppc_ptrace_getregs.32 
Start of child.
Child pid: 9940
Child stopped by signal 5
Test PPC_PTRACE_GETREGS:
ERROR: Parent's GETREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's GETREGS succeeded, reversed arg order
Test PPC_PTRACE_GETFPREGS:
ERROR: Parent's GETFPREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's GETFPREGS succeeded, reversed arg order
Test PPC_PTRACE_SETREGS:
ERROR: Parent's SETREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's SETREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_SETFPREGS:
ERROR: Parent's SETFPREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's SETFPREGS succeeded, reversed arg order

We find that ptrace operations are succeeded with reversed argument order,
except PPC_PTRACE_SETREGS.

------------------------------------------------------------------------------------------------
Yao,
 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.
==============================================================================
#ifdef CONFIG_PPC64
        case PPC_PTRACE_GETREGS: { /* Get GPRs 0 - 31. */
...
                unsigned long __user *tmp = (unsigned long __user *)addr;
                                                                                
                for (i = 0; i < 32; i++) {
                        ret = put_user(*reg, tmp);
                        if (ret)
                                break;
==============================================================================
whereas for i386 architecture these registers get copied to data variable:
==============================================================================
unsigned long __user *datap = (unsigned long __user *)data;
...
case PTRACE_GETREGS: { /* Get all gp regs from the child. */
...
                for ( i = 0; i < FRAME_SIZE*sizeof(long); i += sizeof(long) ) {
                        __put_user(getreg(child, i), datap);
                        datap++;
                }
                ret = 0;
                break;
===============================================================================

The prepared test patch is for aligning
PPC_PTRACE_*REGS options with the general format of ptrace.
Rgds, Supriya

---------------------------------------------------------------------------------------------

I code a patch with your patch as a reference, and rebuild the kernel.	I find
that this patch works.	Here are some steps,

1) rpmbuild -bp SPECS/kernel-2.6.spec
2) Apply this patch, and cd to BUILD/kernel-2.6.17/linux-2.6.17.ppc64
3) cp ./boot/config-2.6.17-1.2630.fc6 .config 
4) make oldconfig & make
5) make modules_install
6) cp vmlinux.strip /boot/vmlinuz-qiyao
7) cp System.map /boot/System.map-qiyao
8) mkinitrd initrd-qiyao 2.6.17-prep
9) edit /etc/yaboot.conf to add kernel image and initrd

Reboot machine,

[qiyao@plinuxt6 ~/ptrace]$ uname -a
Linux plinuxt6 2.6.17-prep #1 SMP Sun Oct 22 22:32:54 EDT 2006 ppc64 ppc64
ppc64 GNU/Linux

And run the test case,
[qiyao@plinuxt6 ~/ptrace]$ gcc test-ppc_ptrace_getregs.c -o
test-ppc_ptrace_getregs.32 
[qiyao@plinuxt6 ~/ptrace]$ gcc -m64 test-ppc_ptrace_getregs.c -o
test-ppc_ptrace_getregs.64
[qiyao@plinuxt6 ~/ptrace]$ ./test-ppc_ptrace_getregs.32 
Start of child.
Child pid: 2877
Child stopped by signal 5
Test PPC_PTRACE_GETREGS:
OK: Parent's GETREGS succeeded, 'correct' arg order
ERROR: Parent's GETREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_GETFPREGS:
OK: Parent's GETFPREGS succeeded, 'correct' arg order
ERROR: Parent's GETFPREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_SETREGS:
ERROR: Parent's SETREGS failed with the 'correct' arg order: Bad address
ERROR: Parent's SETREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_SETFPREGS:
OK: Parent's SETFPREGS succeeded, 'correct' arg order
ERROR: Parent's SETFPREGS failed with reversed arg order: Bad address
[qiyao@plinuxt6 ~/ptrace]$ ./test-ppc_ptrace_getregs.64 
Start of child.
Child pid: 2879
Child stopped by signal 5
Test PPC_PTRACE_GETREGS:
OK: Parent's GETREGS succeeded, 'correct' arg order
ERROR: Parent's GETREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_GETFPREGS:
OK: Parent's GETFPREGS succeeded, 'correct' arg order
ERROR: Parent's GETFPREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_SETREGS:
OK: Parent's SETREGS succeeded, 'correct' arg order
ERROR: Parent's SETREGS failed with reversed arg order: Bad address
Test PPC_PTRACE_SETFPREGS:
OK: Parent's SETFPREGS succeeded, 'correct' arg order
ERROR: Parent's SETFPREGS failed with reversed arg order: Bad address

---------------------------------------------------------------------------------------------
Red Hat,

Mirroring this bug for your review of the patch

Patch information:

Problem:
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.

Arch: PPC-64 
Kernel: 2.6.17-1.2630.fc6

Test Case: <Attached>
Test Results : Succesful, see above
Patch against FC6: <Attached>

Submission to the community: In progress.

-thanks.

Comment 1 IBM Bug Proxy 2006-10-23 16:14:18 UTC
Created attachment 139141 [details]
Test case

Test caseCompile this case in 64-bit mode.

Comment 2 IBM Bug Proxy 2006-10-23 16:14:49 UTC
Created attachment 139142 [details]
Patch against FC6 kernel

Patch against FC6 kernel

Comment 3 IBM Bug Proxy 2006-10-25 11:05:57 UTC
----- Additional Comments From skannery.com  2006-10-25 07:04 EDT -------
Patch submitted in LKML: http://lkml.org/lkml/2006/10/25/69 

Comment 4 David Woodhouse 2006-11-22 18:36:48 UTC
Patch didn't receive much comment. Have forwarded to linux-ppc list.

Comment 5 Andrew Cagney 2006-11-22 19:35:12 UTC
FRYSK (and GDB) doesn't use PTRACE_GET* (it's implementation was missing),
except for the altivec registers and there the correct parameter order appears
to be used:

  ret = ptrace (PTRACE_GETVRREGS, tid, 0, &regs);


Comment 6 IBM Bug Proxy 2006-11-23 12:05:46 UTC
----- Additional Comments From qiyao.com  2006-11-23 06:59 EDT -------
Hi, Andrew,

The reason I find this bug and code this patch is that I find
PTRACE_{GET,SET}REGS, PTRACE_{GET,SET}FPREGS is used in frysk/sys/cni/Ptrace.cxx,

 69 /* There structures and constants are x86-specific.  */
 70 static RegisterSetParams regSetParams[] =
 71   {
 72 #if defined(__i386__)|| defined(__x86_64__)
 73    {sizeof(user_regs_struct), PTRACE_GETREGS, PTRACE_SETREGS},
 74    {sizeof(user_fpregs_struct), PTRACE_GETFPREGS, PTRACE_SETFPREGS},
 75 #if defined(__i386__)
 76    {sizeof(user_fpxregs_struct), PTRACE_GETFPXREGS, PTRACE_SETFPXREGS},
 77 #endif
 78 #endif
 79   };


And, peekRegisters and pokeRegisers read/write data via "data", instead of
"address".

223 void
224 frysk::sys::Ptrace::peekRegisters(jint registerSet, jint pid, jbyteArray data)
225 {
226   _callPtrace(regSetParams[registerSet].peekRequest, pid, 0,
227               (long)elements(data), "ptrace.peekRegisters");
228 }
229
230 void
231 frysk::sys::Ptrace::pokeRegisters(jint registerSet, jint pid, jbyteArray data)
232 {
233   _callPtrace(regSetParams[registerSet].pokeRequest, pid, 0,
234               (long)elements(data), "ptrace.pokeRegisters");
235 }

If we do not fix this defect in kernel, {peek,poke}Registers may not be
implemented consistently on x86,x86-64, and ppc64.  We need to swtich the order
of the last two arguments explictly in {peek,poke}Registers if the platform is
ppc64. 

Comment 7 Andrew Cagney 2006-11-23 15:09:05 UTC
Oop, so thats how IBM did it.  Don't worry about that, fix the bug.


Comment 8 Andrew Cagney 2006-11-23 15:46:35 UTC
(In reply to comment #7)
> Oop, so thats how IBM did it.  Don't worry about that, fix the bug.
> 

Sorry, ignore my last comment.

Yes the bug should just be fixed; I checked GDB and it doesn't use that
interface on PPC so nothing shipped can break.

For frysk, yes it should switch to use GETREGS.  I've added two frysk bugs:
  http://sourceware.org/bugzilla/show_bug.cgi?id=3576
     kernel bug
  http://sourceware.org/bugzilla/show_bug.cgi?id=3577
     ppc use SETREGS
you might want to grab one or both of them.


Comment 9 Chuck Ebbert 2007-06-22 15:13:05 UTC
What happened to the kernel patch to change the order of the arguments? Was it
rejected? 2.6.22-rc5 still has the wrong order...

Comment 10 Jon Stanley 2007-12-31 02:35:06 UTC
Since FC6 is no longer maintained and this issue is mentioned as applying to
2.6.22 upstream, I'll move this to F7.

Comment 11 Jon Stanley 2008-03-09 06:25:52 UTC
Is this still an issue or has it been patched upstream?

Comment 12 Brian Powell 2008-04-25 03:48:30 UTC
Note that maintenance for Fedora 7 will end 30 days after the GA of Fedora 9.

Comment 13 Brian Powell 2008-04-25 04:02:51 UTC
The information we've requested above is required in order
to review this problem report further and diagnose/fix the
issue if it is still present.  Since there have not been any
updates to the report since thirty (30) days or more since we
requested additional information, we're assuming the problem
is either no longer present in the current Fedora release, or
that there is no longer any interest in tracking the problem.

Setting status to "CLOSED INSUFFICIENT_DATA".  If you still
experience this problem after updating to our latest Fedora
release and can provide the information previously requested, 
please feel free to reopen the bug report.

Thank you in advance.

Note that maintenance for Fedora 7 will end 30 days after the GA of Fedora 9.

Comment 14 IBM Bug Proxy 2008-08-08 07:10:33 UTC
Since FC6 is no longer maintained and this issue is mentioned as applying to
2.6.22 upstream, I'll move this to F7.

Is this still an issue or has it been patched upstream?

Note that maintenance for Fedora 7 will end 30 days after the GA of Fedora 9.

The information we've requested above is required in order
to review this problem report further and diagnose/fix the
issue if it is still present.  Since there have not been any
updates to the report since thirty (30) days or more since we
requested additional information, we're assuming the problem
is either no longer present in the current Fedora release, or
that there is no longer any interest in tracking the problem.

Setting status to "CLOSED INSUFFICIENT_DATA".  If you still
experience this problem after updating to our latest Fedora
release and can provide the information previously requested,
please feel free to reopen the bug report.

Thank you in advance.

Note that maintenance for Fedora 7 will end 30 days after the GA of Fedora 9.


Closing from IBM side too. If any further situation arises where such a code
change is needed, will reopen and re-evaluate.


Note You need to log in before you can comment on or make changes to this bug.