[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[patches] NPTL setxid race
- To: patches@xxxxxxxxxx
- Subject: [patches] NPTL setxid race
- From: Daniel Jacobowitz <dan@xxxxxxxxxxxxxxxx>
- Date: Sat, 23 Feb 2008 22:05:13 -0500
I've committed the attached patches, which fix glibc PR nptl/3270.
The lll-private version went on trunk and the EGLIBC 2.7 branch; the
original version went on EGLIBC 2.5 and EGLIBC 2.6. Joseph approved
this off-list and I ran regression tests for x86_64-linux. For more
information, see the glibc PR in bugzilla.
--
Daniel Jacobowitz
CodeSourcery
2008-02-23 Daniel Jacobowitz <dan@xxxxxxxxxxxxxxxx>
PR nptl/3270
nptl/
* allocatestack.c (setxid_mark_thread, setxid_unmark_thread): New.
(setxid_signal_thread): Return a successful signal indicator. Just
skip threads without SETXID_BITMASK.
(__nptl_setxid): Use separate marking and unmarking loops. Repeat
signalling if necessary.
* init.c (sighandler_setxid): Use atomic operations for
cancelhandling. Wake __nptl_setxid last.
Index: allocatestack.c
===================================================================
RCS file: /cvs/glibc/libc/nptl/allocatestack.c,v
retrieving revision 1.64
diff -u -p -r1.64 allocatestack.c
--- allocatestack.c 23 Aug 2006 17:39:47 -0000 1.64
+++ allocatestack.c 27 Sep 2006 14:47:46 -0000
@@ -852,22 +852,53 @@ __find_thread_by_id (pid_t tid)
static void
internal_function
-setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
+setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
{
- if (! IS_DETACHED (t))
+ int ch;
+
+ /* Don't let the thread exit before the setxid handler runs. */
+ t->setxid_futex = 0;
+
+ do
{
- int ch;
- do
- {
- ch = t->cancelhandling;
+ ch = t->cancelhandling;
- /* If the thread is exiting right now, ignore it. */
- if ((ch & EXITING_BITMASK) != 0)
- return;
- }
- while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
- ch | SETXID_BITMASK, ch));
+ /* If the thread is exiting right now, ignore it. */
+ if ((ch & EXITING_BITMASK) != 0)
+ return;
+ }
+ while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
+ ch | SETXID_BITMASK, ch));
+}
+
+
+static void
+internal_function
+setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t)
+{
+ int ch;
+
+ do
+ {
+ ch = t->cancelhandling;
+ if ((ch & SETXID_BITMASK) == 0)
+ return;
}
+ while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
+ ch & ~SETXID_BITMASK, ch));
+
+ /* Release the futex just in case. */
+ t->setxid_futex = 1;
+ lll_futex_wake (&t->setxid_futex, 1);
+}
+
+
+static int
+internal_function
+setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
+{
+ if ((t->cancelhandling & SETXID_BITMASK) == 0)
+ return 0;
int val;
INTERNAL_SYSCALL_DECL (err);
@@ -884,8 +915,14 @@ setxid_signal_thread (struct xid_command
val = INTERNAL_SYSCALL (tkill, err, 2, t->tid, SIGSETXID);
#endif
+ /* If this failed, it must have had not started yet or else exited. */
if (!INTERNAL_SYSCALL_ERROR_P (val, err))
- atomic_increment (&cmdp->cntr);
+ {
+ atomic_increment (&cmdp->cntr);
+ return 1;
+ }
+ else
+ return 0;
}
@@ -893,6 +930,7 @@ int
attribute_hidden
__nptl_setxid (struct xid_command *cmdp)
{
+ int signalled;
int result;
lll_lock (stack_cache_lock);
@@ -909,7 +947,7 @@ __nptl_setxid (struct xid_command *cmdp)
if (t == self)
continue;
- setxid_signal_thread (cmdp, t);
+ setxid_mark_thread (cmdp, t);
}
/* Now the list with threads using user-allocated stacks. */
@@ -919,14 +957,61 @@ __nptl_setxid (struct xid_command *cmdp)
if (t == self)
continue;
- setxid_signal_thread (cmdp, t);
+ setxid_mark_thread (cmdp, t);
}
- int cur = cmdp->cntr;
- while (cur != 0)
+ /* Iterate until we don't succeed in signalling anyone. That means
+ we have gotten all running threads, and their children will be
+ automatically correct once started. */
+ do
{
- lll_futex_wait (&cmdp->cntr, cur);
- cur = cmdp->cntr;
+ signalled = 0;
+
+ list_for_each (runp, &stack_used)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ signalled += setxid_signal_thread (cmdp, t);
+ }
+
+ list_for_each (runp, &__stack_user)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ signalled += setxid_signal_thread (cmdp, t);
+ }
+
+ int cur = cmdp->cntr;
+ while (cur != 0)
+ {
+ lll_futex_wait (&cmdp->cntr, cur);
+ cur = cmdp->cntr;
+ }
+ }
+ while (signalled != 0);
+
+ /* Clean up flags, so that no thread blocks during exit waiting
+ for a signal which will never come. */
+ list_for_each (runp, &stack_used)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ setxid_unmark_thread (cmdp, t);
+ }
+
+ list_for_each (runp, &__stack_user)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ setxid_unmark_thread (cmdp, t);
}
/* This must be last, otherwise the current thread might not have
Index: init.c
===================================================================
RCS file: /cvs/glibc/libc/nptl/init.c,v
retrieving revision 1.59
diff -u -p -r1.59 init.c
--- init.c 23 Aug 2006 17:41:31 -0000 1.59
+++ init.c 27 Sep 2006 14:47:46 -0000
@@ -236,17 +236,23 @@ sighandler_setxid (int sig, siginfo_t *s
INTERNAL_SYSCALL_NCS (__xidcmd->syscall_no, err, 3, __xidcmd->id[0],
__xidcmd->id[1], __xidcmd->id[2]);
- if (atomic_decrement_val (&__xidcmd->cntr) == 0)
- lll_futex_wake (&__xidcmd->cntr, 1);
-
/* Reset the SETXID flag. */
struct pthread *self = THREAD_SELF;
- int flags = THREAD_GETMEM (self, cancelhandling);
- THREAD_SETMEM (self, cancelhandling, flags & ~SETXID_BITMASK);
+ int flags, newval;
+ do
+ {
+ flags = THREAD_GETMEM (self, cancelhandling);
+ newval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
+ flags & ~SETXID_BITMASK, flags);
+ }
+ while (flags != newval);
/* And release the futex. */
self->setxid_futex = 1;
lll_futex_wake (&self->setxid_futex, 1);
+
+ if (atomic_decrement_val (&__xidcmd->cntr) == 0)
+ lll_futex_wake (&__xidcmd->cntr, 1);
}
2008-02-23 Daniel Jacobowitz <dan@xxxxxxxxxxxxxxxx>
PR nptl/3270
nptl/
* allocatestack.c (setxid_mark_thread, setxid_unmark_thread): New.
(setxid_signal_thread): Return a successful signal indicator. Just
skip threads without SETXID_BITMASK.
(__nptl_setxid): Use separate marking and unmarking loops. Repeat
signalling if necessary.
* init.c (sighandler_setxid): Use atomic operations for
cancelhandling. Wake __nptl_setxid last.
Index: nptl/init.c
===================================================================
--- nptl/init.c (revision 5284)
+++ nptl/init.c (working copy)
@@ -221,12 +221,21 @@ sighandler_setxid (int sig, siginfo_t *s
/* Reset the SETXID flag. */
struct pthread *self = THREAD_SELF;
- int flags = THREAD_GETMEM (self, cancelhandling);
- THREAD_SETMEM (self, cancelhandling, flags & ~SETXID_BITMASK);
+ int flags, newval;
+ do
+ {
+ flags = THREAD_GETMEM (self, cancelhandling);
+ newval = THREAD_ATOMIC_CMPXCHG_VAL (self, cancelhandling,
+ flags & ~SETXID_BITMASK, flags);
+ }
+ while (flags != newval);
/* And release the futex. */
self->setxid_futex = 1;
lll_futex_wake (&self->setxid_futex, 1, LLL_PRIVATE);
+
+ if (atomic_decrement_val (&__xidcmd->cntr) == 0)
+ lll_futex_wake (&__xidcmd->cntr, 1, LLL_PRIVATE);
}
Index: nptl/allocatestack.c
===================================================================
--- nptl/allocatestack.c (revision 5284)
+++ nptl/allocatestack.c (working copy)
@@ -904,22 +904,53 @@ __find_thread_by_id (pid_t tid)
static void
internal_function
-setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
+setxid_mark_thread (struct xid_command *cmdp, struct pthread *t)
{
- if (! IS_DETACHED (t))
+ int ch;
+
+ /* Don't let the thread exit before the setxid handler runs. */
+ t->setxid_futex = 0;
+
+ do
{
- int ch;
- do
- {
- ch = t->cancelhandling;
+ ch = t->cancelhandling;
- /* If the thread is exiting right now, ignore it. */
- if ((ch & EXITING_BITMASK) != 0)
- return;
- }
- while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
- ch | SETXID_BITMASK, ch));
+ /* If the thread is exiting right now, ignore it. */
+ if ((ch & EXITING_BITMASK) != 0)
+ return;
+ }
+ while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
+ ch | SETXID_BITMASK, ch));
+}
+
+
+static void
+internal_function
+setxid_unmark_thread (struct xid_command *cmdp, struct pthread *t)
+{
+ int ch;
+
+ do
+ {
+ ch = t->cancelhandling;
+ if ((ch & SETXID_BITMASK) == 0)
+ return;
}
+ while (atomic_compare_and_exchange_bool_acq (&t->cancelhandling,
+ ch & ~SETXID_BITMASK, ch));
+
+ /* Release the futex just in case. */
+ t->setxid_futex = 1;
+ lll_futex_wake (&t->setxid_futex, 1, LLL_PRIVATE);
+}
+
+
+static int
+internal_function
+setxid_signal_thread (struct xid_command *cmdp, struct pthread *t)
+{
+ if ((t->cancelhandling & SETXID_BITMASK) == 0)
+ return 0;
int val;
INTERNAL_SYSCALL_DECL (err);
@@ -936,8 +967,14 @@ setxid_signal_thread (struct xid_command
val = INTERNAL_SYSCALL (tkill, err, 2, t->tid, SIGSETXID);
#endif
+ /* If this failed, it must have had not started yet or else exited. */
if (!INTERNAL_SYSCALL_ERROR_P (val, err))
- atomic_increment (&cmdp->cntr);
+ {
+ atomic_increment (&cmdp->cntr);
+ return 1;
+ }
+ else
+ return 0;
}
@@ -945,6 +982,7 @@ int
attribute_hidden
__nptl_setxid (struct xid_command *cmdp)
{
+ int signalled;
int result;
lll_lock (stack_cache_lock, LLL_PRIVATE);
@@ -961,7 +999,7 @@ __nptl_setxid (struct xid_command *cmdp)
if (t == self)
continue;
- setxid_signal_thread (cmdp, t);
+ setxid_mark_thread (cmdp, t);
}
/* Now the list with threads using user-allocated stacks. */
@@ -971,14 +1009,61 @@ __nptl_setxid (struct xid_command *cmdp)
if (t == self)
continue;
- setxid_signal_thread (cmdp, t);
+ setxid_mark_thread (cmdp, t);
}
- int cur = cmdp->cntr;
- while (cur != 0)
+ /* Iterate until we don't succeed in signalling anyone. That means
+ we have gotten all running threads, and their children will be
+ automatically correct once started. */
+ do
{
- lll_futex_wait (&cmdp->cntr, cur, LLL_PRIVATE);
- cur = cmdp->cntr;
+ signalled = 0;
+
+ list_for_each (runp, &stack_used)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ signalled += setxid_signal_thread (cmdp, t);
+ }
+
+ list_for_each (runp, &__stack_user)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ signalled += setxid_signal_thread (cmdp, t);
+ }
+
+ int cur = cmdp->cntr;
+ while (cur != 0)
+ {
+ lll_futex_wait (&cmdp->cntr, cur, LLL_PRIVATE);
+ cur = cmdp->cntr;
+ }
+ }
+ while (signalled != 0);
+
+ /* Clean up flags, so that no thread blocks during exit waiting
+ for a signal which will never come. */
+ list_for_each (runp, &stack_used)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ setxid_unmark_thread (cmdp, t);
+ }
+
+ list_for_each (runp, &__stack_user)
+ {
+ struct pthread *t = list_entry (runp, struct pthread, list);
+ if (t == self)
+ continue;
+
+ setxid_unmark_thread (cmdp, t);
}
/* This must be last, otherwise the current thread might not have