strace sys_clone bug on x86-32

enh enh at google.com
Sun Apr 6 19:41:17 UTC 2014


here's the simplest fix:

commit 93f15b777ab8362d96c51db3372cfc47b8686abd
Author: Elliott Hughes <enh at google.com>
Date:   Sat Apr 5 11:56:17 2014 -0700

    Fix clone(2) argument order for 32-bit processes on x86-64.

    Example output from a bionic pthread unit test afterwards:

      clone(child_stack=0xe82f1424, flags=CLONE_VM|CLONE_FS|CLONE_FILES|
            CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|
            CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
            parent_tidptr=0xe82f1ba8, tls=0xff841750, child_tidptr=0xe82f1ba8) =
                3121

    Without this patch, strace claims that parent_tidptr == tls, which is
    clearly wrong. (It's expected that parent_tidptr == child_tidptr.)

    Change-Id: I00988a4b68e75d7ac361e356c64fd2b61e4a2940
    Signed-off-by: Elliott Hughes <enh at google.com>

diff --git a/process.c b/process.c
index e3837da..c9baf0e 100644
--- a/process.c
+++ b/process.c
@@ -512,8 +512,14 @@ extern void print_ldt_entry();
 # define ARG_PTID 2
 # define ARG_CTID 3
 # define ARG_TLS 4
-#elif defined X86_64 || defined X32 || defined ALPHA || defined TILE \
-   || defined OR1K
+#elif defined X86_64
+/* 32-bit processes have the last two arguments flipped. */
+# define ARG_FLAGS 0
+# define ARG_STACK 1
+# define ARG_PTID 2
+# define ARG_CTID ((current_personality != 1) ? 3 : 4)
+# define ARG_TLS ((current_personality != 1) ? 4 : 3)
+#elif defined X32 || defined ALPHA || defined TILE || defined OR1K
 # define ARG_FLAGS 0
 # define ARG_STACK 1
 # define ARG_PTID 2


if you want to get fancy, you can also decode the LDT user_desc
entries (like you would if you were using an x86 strace to trace an
x86 process):

diff --git a/mem.c b/mem.c
index 267773c..95ea364 100644
--- a/mem.c
+++ b/mem.c
@@ -33,7 +33,7 @@
 #include "defs.h"
 #include <asm/mman.h>
 #include <sys/mman.h>
-#if defined(I386)
+#if defined(I386) || defined(X86_64)
 # include <asm/ldt.h>
 # ifdef HAVE_STRUCT_USER_DESC
 #  define modify_ldt_ldt_s user_desc
@@ -543,7 +543,7 @@ sys_getpagesize(struct tcb *tcp)
 }
 #endif

-#if defined(I386)
+#if defined(I386) || defined(X86_64)
 void
 print_ldt_entry(struct modify_ldt_ldt_s *ldt_entry)
 {
diff --git a/process.c b/process.c
index c9baf0e..12be785 100644
--- a/process.c
+++ b/process.c
@@ -491,7 +491,7 @@ static const struct xlat clone_flags[] = {
  XLAT_END
 };

-#ifdef I386
+#if defined I386 || defined X86_64
 # include <asm/ldt.h>
 #  ifdef HAVE_STRUCT_USER_DESC
 #   define modify_ldt_ldt_s user_desc
@@ -556,18 +556,27 @@ sys_clone(struct tcb *tcp)
  if (flags & CLONE_PARENT_SETTID)
  tprintf(", parent_tidptr=%#lx", tcp->u_arg[ARG_PTID]);
  if (flags & CLONE_SETTLS) {
-#ifdef I386
- struct modify_ldt_ldt_s copy;
- if (umove(tcp, tcp->u_arg[ARG_TLS], &copy) != -1) {
- tprintf(", {entry_number:%d, ",
- copy.entry_number);
- if (!verbose(tcp))
- tprints("...}");
- else
- print_ldt_entry(&copy);
+ int print_raw_tls = 1;
+#if defined I386 || defined X86_64
+ /* On x86 Linux, the TLS argument is a pointer to a
+ * struct user_desc, not the TLS itself. */
+#if defined X86_64
+ if (current_personality == 1)
+#endif
+ {
+ struct modify_ldt_ldt_s copy;
+ if (umove(tcp, tcp->u_arg[ARG_TLS], &copy) != -1) {
+ tprintf(", {entry_number:%d, ",
+ copy.entry_number);
+ if (!verbose(tcp))
+ tprints("...}");
+ else
+ print_ldt_entry(&copy);
+ print_raw_tls = 0;
+ }
  }
- else
 #endif
+ if (print_raw_tls)
  tprintf(", tls=%#lx", tcp->u_arg[ARG_TLS]);
  }
  if (flags & (CLONE_CHILD_SETTID|CLONE_CHILD_CLEARTID))


here's an example of the clone(2) underlying a 32-bit glibc
pthread_create (bionic doesn't use CLONE_SETTLS on x86):

clone(child_stack=0xf75bf424,
flags=CLONE_VM|CLONE_FS|CLONE_FILES|CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
parent_tidptr=0xf75bfba8, {entry_number:12, base_addr:0xf75bfb40,
limit:1048575, seg_32bit:1, contents:0, read_exec_only:0,
limit_in_pages:1, seg_not_present:0, useable:1},
child_tidptr=0xf75bfba8) = 9270

(without the second patch you get tls=0xaddr, where the address is the
address of the user_desc, not the actual TLS address.)

On Tue, Nov 26, 2013 at 4:08 PM, enh <enh at google.com> wrote:
> strace's process.c makes a compile-time decision about what order the
> arguments to sys_clone are in; this doesn't work on x86/x86-64 because
> you don't know until runtime whether you're tracing a 32-bit or 64-bit
> process.
>
> this means that strace on a 32-bit process that calls pthread_create
> shows the sys_clone tls and child tid arguments flipped.
>
> (i maintain Android's C library and spent today debugging Android's
> pthread_create for x86-32 on an x86-64 desktop, so i may be the first
> and last person ever to hit this bug, but i thought i'd report it
> anyway.)
>
>  -e



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Java i18n/JNI/NIO, or bionic questions? Mail me/drop by/add me as a reviewer.
-------------- next part --------------
commit 93f15b777ab8362d96c51db3372cfc47b8686abd
Author: Elliott Hughes <enh at google.com>
Date:   Sat Apr 5 11:56:17 2014 -0700

    Fix clone(2) argument order for 32-bit processes on x86-64.
    
    Example output from a bionic pthread unit test afterwards:
    
      clone(child_stack=0xe82f1424, flags=CLONE_VM|CLONE_FS|CLONE_FILES|
            CLONE_SIGHAND|CLONE_THREAD|CLONE_SYSVSEM|CLONE_SETTLS|
            CLONE_PARENT_SETTID|CLONE_CHILD_CLEARTID,
            parent_tidptr=0xe82f1ba8, tls=0xff841750, child_tidptr=0xe82f1ba8) =
                3121
    
    Without this patch, strace claims that parent_tidptr == tls, which is
    clearly wrong. (It's expected that parent_tidptr == child_tidptr.)
    
    Change-Id: I00988a4b68e75d7ac361e356c64fd2b61e4a2940
    Signed-off-by: Elliott Hughes <enh at google.com>

diff --git a/process.c b/process.c
index e3837da..c9baf0e 100644
--- a/process.c
+++ b/process.c
@@ -512,8 +512,14 @@ extern void print_ldt_entry();
 # define ARG_PTID	2
 # define ARG_CTID	3
 # define ARG_TLS	4
-#elif defined X86_64 || defined X32 || defined ALPHA || defined TILE \
-   || defined OR1K
+#elif defined X86_64
+/* 32-bit processes have the last two arguments flipped. */
+# define ARG_FLAGS	0
+# define ARG_STACK	1
+# define ARG_PTID	2
+# define ARG_CTID	((current_personality != 1) ? 3 : 4)
+# define ARG_TLS	((current_personality != 1) ? 4 : 3)
+#elif defined X32 || defined ALPHA || defined TILE || defined OR1K
 # define ARG_FLAGS	0
 # define ARG_STACK	1
 # define ARG_PTID	2
-------------- next part --------------
diff --git a/mem.c b/mem.c
index 267773c..95ea364 100644
--- a/mem.c
+++ b/mem.c
@@ -33,7 +33,7 @@
 #include "defs.h"
 #include <asm/mman.h>
 #include <sys/mman.h>
-#if defined(I386)
+#if defined(I386) || defined(X86_64)
 # include <asm/ldt.h>
 # ifdef HAVE_STRUCT_USER_DESC
 #  define modify_ldt_ldt_s user_desc
@@ -543,7 +543,7 @@ sys_getpagesize(struct tcb *tcp)
 }
 #endif
 
-#if defined(I386)
+#if defined(I386) || defined(X86_64)
 void
 print_ldt_entry(struct modify_ldt_ldt_s *ldt_entry)
 {
diff --git a/process.c b/process.c
index c9baf0e..82cff25 100644
--- a/process.c
+++ b/process.c
@@ -491,7 +491,7 @@ static const struct xlat clone_flags[] = {
 	XLAT_END
 };
 
-#ifdef I386
+#if defined I386 || defined X86_64
 # include <asm/ldt.h>
 #  ifdef HAVE_STRUCT_USER_DESC
 #   define modify_ldt_ldt_s user_desc
@@ -556,15 +556,20 @@ sys_clone(struct tcb *tcp)
 		if (flags & CLONE_PARENT_SETTID)
 			tprintf(", parent_tidptr=%#lx", tcp->u_arg[ARG_PTID]);
 		if (flags & CLONE_SETTLS) {
-#ifdef I386
-			struct modify_ldt_ldt_s copy;
-			if (umove(tcp, tcp->u_arg[ARG_TLS], &copy) != -1) {
-				tprintf(", {entry_number:%d, ",
-					copy.entry_number);
-				if (!verbose(tcp))
-					tprints("...}");
-				else
-					print_ldt_entry(&copy);
+#if defined I386 || defined X86_64
+#if defined X86_64
+			if (current_personality == 1)
+#endif
+			{
+				struct modify_ldt_ldt_s copy;
+				if (umove(tcp, tcp->u_arg[ARG_TLS], &copy) != -1) {
+					tprintf(", {entry_number:%d, ",
+						copy.entry_number);
+					if (!verbose(tcp))
+						tprints("...}");
+					else
+						print_ldt_entry(&copy);
+				}
 			}
 			else
 #endif


More information about the Strace-devel mailing list