| 1 |
From nobody Mon Sep 17 00:00:00 2001 |
| 2 |
Subject: [PATCH] Fix fs/exec.c:788 (de_thread()) BUG_ON |
| 3 |
From: Alexander Nyberg <alexn@telia.com> |
| 4 |
Date: Wed Sep 14 18:54:06 2005 +0200 |
| 5 |
|
| 6 |
It turns out that the BUG_ON() in fs/exec.c: de_thread() is unreliable |
| 7 |
and can trigger due to the test itself being racy. |
| 8 |
|
| 9 |
de_thread() does |
| 10 |
while (atomic_read(&sig->count) > count) { |
| 11 |
} |
| 12 |
..... |
| 13 |
..... |
| 14 |
BUG_ON(!thread_group_empty(current)); |
| 15 |
|
| 16 |
but release_task does |
| 17 |
write_lock_irq(&tasklist_lock) |
| 18 |
__exit_signal |
| 19 |
(this is where atomic_dec(&sig->count) is run) |
| 20 |
__exit_sighand |
| 21 |
__unhash_process |
| 22 |
takes write lock on tasklist_lock |
| 23 |
remove itself out of PIDTYPE_TGID list |
| 24 |
write_unlock_irq(&tasklist_lock) |
| 25 |
|
| 26 |
so there's a clear (although small) window between the |
| 27 |
atomic_dec(&sig->count) and the actual PIDTYPE_TGID unhashing of the |
| 28 |
thread. |
| 29 |
|
| 30 |
And actually there is no need for all threads to have exited at this |
| 31 |
point, so we simply kill the BUG_ON. |
| 32 |
|
| 33 |
Big thanks to Marc Lehmann who provided the test-case. |
| 34 |
|
| 35 |
Fixes Bug 5170 (http://bugme.osdl.org/show_bug.cgi?id=5170) |
| 36 |
|
| 37 |
Signed-off-by: Alexander Nyberg <alexn@telia.com> |
| 38 |
Cc: Roland McGrath <roland@redhat.com> |
| 39 |
Cc: Andrew Morton <akpm@osdl.org> |
| 40 |
Cc: Ingo Molnar <mingo@elte.hu> |
| 41 |
Acked-by: Andi Kleen <ak@suse.de> |
| 42 |
Signed-off-by: Linus Torvalds <torvalds@osdl.org> |
| 43 |
Signed-off-by: Chris Wright <chrisw@osdl.org> |
| 44 |
--- |
| 45 |
fs/exec.c | 5 ++--- |
| 46 |
1 files changed, 2 insertions(+), 3 deletions(-) |
| 47 |
|
| 48 |
Index: linux-2.6.13.y/fs/exec.c |
| 49 |
=================================================================== |
| 50 |
--- linux-2.6.13.y.orig/fs/exec.c |
| 51 |
+++ linux-2.6.13.y/fs/exec.c |
| 52 |
@@ -745,8 +745,8 @@ static inline int de_thread(struct task_ |
| 53 |
} |
| 54 |
|
| 55 |
/* |
| 56 |
- * Now there are really no other threads at all, |
| 57 |
- * so it's safe to stop telling them to kill themselves. |
| 58 |
+ * There may be one thread left which is just exiting, |
| 59 |
+ * but it's safe to stop telling the group to kill themselves. |
| 60 |
*/ |
| 61 |
sig->flags = 0; |
| 62 |
|
| 63 |
@@ -785,7 +785,6 @@ no_thread_group: |
| 64 |
kmem_cache_free(sighand_cachep, oldsighand); |
| 65 |
} |
| 66 |
|
| 67 |
- BUG_ON(!thread_group_empty(current)); |
| 68 |
BUG_ON(!thread_group_leader(current)); |
| 69 |
return 0; |
| 70 |
} |