[PATCH] Fix setxid race with thread creation

[PATCH] Fix setxid race with thread creation

[Andreas Schwab]

When setxid runs in the time window between allocating a thread stack
and cloning the new thread this thread does not receive the setxid
signal and remains at the wrong xid.

Andreas.

2010-03-02  Andreas Schwab  <schwab@redhat.com>

	* allocatestack.c (get_cached_stack): Initialize setxid_futex to -1.
	(allocate_stack): Likewise.
	(setxid_mark_thread): Wait for setxid_futex != -1.
	* sysdeps/pthread/createthread.c (do_clone): Set setxid_futex
	after cloning.

diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 3c3585f..88f9574 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -213,6 +213,9 @@ get_cached_stack (size_t *sizep, void **memp)
       return NULL;
     }
 
+  /* Don't allow setxid until cloned.  */
+  result->setxid_futex = -1;
+
   /* Dequeue the entry.  */
   stack_list_del (&result->list);
 
@@ -418,6 +421,9 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
       /* The process ID is also the same as that of the caller.  */
       pd->pid = THREAD_GETMEM (THREAD_SELF, pid);
 
+      /* Don't allow setxid until cloned.  */
+      pd->setxid_futex = -1;
+
       /* Allocate the DTV for this thread.  */
       if (_dl_allocate_tls (TLS_TPADJ (pd)) == NULL)
 	{
@@ -557,6 +563,9 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
 	  /* The process ID is also the same as that of the caller.  */
 	  pd->pid = THREAD_GETMEM (THREAD_SELF, pid);
 
+	  /* Don't allow setxid until cloned.  */
+	  pd->setxid_futex = -1;
+
 	  /* Allocate the DTV for this thread.  */
 	  if (_dl_allocate_tls (TLS_TPADJ (pd)) == NULL)
 	    {
@@ -969,6 +978,10 @@ setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
 {
   int ch;
 
+  /* Wait until this thread is cloned.  */
+  while (t->setxid_futex == -1)
+    lll_futex_wait (&t->setxid_futex, -1, LLL_PRIVATE);
+
   /* Don't let the thread exit before the setxid handler runs.  */
   t->setxid_futex = 0;
 
diff --git a/nptl/sysdeps/pthread/createthread.c b/nptl/sysdeps/pthread/createthread.c
index 66fafe8..d176bad 100644
--- a/nptl/sysdeps/pthread/createthread.c
+++ b/nptl/sysdeps/pthread/createthread.c
@@ -52,6 +52,8 @@ do_clone (struct pthread *pd, const struct pthread_attr *attr,
 	  int clone_flags, int (*fct) (void *), STACK_VARIABLES_PARMS,
 	  int stopped)
 {
+  int rc;
+
 #ifdef PREPARE_CREATE
   PREPARE_CREATE;
 #endif
@@ -72,8 +74,14 @@ do_clone (struct pthread *pd, const struct pthread_attr *attr,
      that cares whether the thread count is correct.  */
   atomic_increment (&__nptl_nthreads);
 
-  if (ARCH_CLONE (fct, STACK_VARIABLES_ARGS, clone_flags,
-		  pd, &pd->tid, TLS_VALUE, &pd->tid) == -1)
+  rc = ARCH_CLONE (fct, STACK_VARIABLES_ARGS, clone_flags,
+		   pd, &pd->tid, TLS_VALUE, &pd->tid);
+
+  /* Allow setxid from now onwards.  */
+  pd->setxid_futex = 0;
+  lll_futex_wake (&pd->setxid_futex, 1, LLL_PRIVATE);
+  
+  if (rc == -1)
     {
       atomic_decrement (&__nptl_nthreads); /* Oops, we lied for a second.  */
 

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] Fix setxid race with thread creation

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I've checked in a modified version of the patch.  There are some
optimizations and a problem with the created thread not waking the
setxid thread.  Please test it.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkuRWoQACgkQ2ijCOnn/RHR86QCfca6jl2lukw/deyorckXJJeXA
7W4An2gIqWqQ+nz8iEZsj89r1iuOtKl+
=qG4B
-----END PGP SIGNATURE-----

Re: [PATCH] Fix setxid race with thread creation

[Andreas Schwab]

Ulrich Drepper <drepper@redhat.com> writes:

> I've checked in a modified version of the patch.  There are some
> optimizations and a problem with the created thread not waking the
> setxid thread.  Please test it.

It does not work at all.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: [PATCH] Fix setxid race with thread creation

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 03/08/2010 05:57 AM, Andreas Schwab wrote:
> It does not work at all.

You have to be more specific.  I ran the test case in the BZ for 4 hours
without any problem.

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEUEARECAAYFAkuV5UIACgkQ2ijCOnn/RHQzIgCXXUZpIGyg86oNgV5+6Cy1rCG4
LgCguccmPa5bJd9qJhkgG/OTfaGBgj0=
=TQTw
-----END PGP SIGNATURE-----

Re: [PATCH] Fix setxid race with thread creation

[Andreas Schwab]

Ulrich Drepper <drepper@redhat.com> writes:

> On 03/08/2010 05:57 AM, Andreas Schwab wrote:
>> It does not work at all.
>
> You have to be more specific.

It does not fix the bug in any way.

> I ran the test case in the BZ for 4 hours without any problem.

Which test case?

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."