| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <87vaqooggs.fsf_-_@xmission.com>
Date: Sat, 01 Apr 2017 00:16:19 -0500
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Oleg Nesterov <oleg@...hat.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Aleksa Sarai <asarai@...e.com>,
Andy Lutomirski <luto@...capital.net>,
Attila Fazekas <afazekas@...hat.com>,
Jann Horn <jann@...jh.net>, Kees Cook <keescook@...omium.org>,
Michal Hocko <mhocko@...nel.org>,
Ulrich Obergfell <uobergfe@...hat.com>,
linux-kernel@...r.kernel.org, linux-api@...r.kernel.org
Subject: [RFC][PATCH 2/2] exec: If possible don't wait for ptraced threads to be reaped
Take advantage of the situation when sighand->count == 1 to only wait
for threads to reach EXIT_ZOMBIE instead of EXIT_DEAD in de_thread.
Only old old linux threading libraries use CLONE_SIGHAND without
CLONE_THREAD. So this situation should be present most of the time.
This allows ptracing through a multi-threaded exec without the danger
of stalling the exec. As historically exec waits for the other
threads to be reaped in de_thread before completing. This is
necessary as it is not safe to unshare the sighand_struct until all of
the other threads in this thread group are reaped, because the lock to
serialize threads in a thread group siglock lives in sighand_struct.
When oldsighand->count == 1 we know that there are no other
users and unsharing the sighand struct in exec is pointless.
This makes it safe to only wait for threads to become zombies
as the siglock won't change during exec and release_task
will use the samve siglock for the old threads as for
the new threads.
Cc: stable@...r.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
---
fs/exec.c | 15 ++++++++++-----
include/linux/sched/signal.h | 2 +-
kernel/exit.c | 13 +++++++++----
kernel/signal.c | 8 ++++++--
4 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 65145a3df065..0fd29342bbe4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1052,6 +1052,7 @@ static int de_thread(struct task_struct *tsk)
struct signal_struct *sig = tsk->signal;
struct sighand_struct *oldsighand = tsk->sighand;
spinlock_t *lock = &oldsighand->siglock;
+ bool may_hang;
if (thread_group_empty(tsk))
goto no_thread_group;
@@ -1069,9 +1070,10 @@ static int de_thread(struct task_struct *tsk)
return -EAGAIN;
}
+ may_hang = atomic_read(&oldsighand->count) != 1;
sig->group_exit_task = tsk;
- sig->notify_count = zap_other_threads(tsk);
- if (!thread_group_leader(tsk))
+ sig->notify_count = zap_other_threads(tsk, may_hang ? 1 : -1);
+ if (may_hang && !thread_group_leader(tsk))
sig->notify_count--;
while (sig->notify_count) {
@@ -1092,9 +1094,10 @@ static int de_thread(struct task_struct *tsk)
if (!thread_group_leader(tsk)) {
struct task_struct *leader = tsk->group_leader;
- for (;;) {
- cgroup_threadgroup_change_begin(tsk);
- write_lock_irq(&tasklist_lock);
+ cgroup_threadgroup_change_begin(tsk);
+ write_lock_irq(&tasklist_lock);
+
+ for (;may_hang;) {
/*
* Do this under tasklist_lock to ensure that
* exit_notify() can't miss ->group_exit_task
@@ -1108,6 +1111,8 @@ static int de_thread(struct task_struct *tsk)
schedule();
if (unlikely(__fatal_signal_pending(tsk)))
goto killed;
+ cgroup_threadgroup_change_begin(tsk);
+ write_lock_irq(&tasklist_lock);
}
/*
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 2cf446704cd4..187a9e980d3a 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -298,7 +298,7 @@ extern __must_check bool do_notify_parent(struct task_struct *, int);
extern void __wake_up_parent(struct task_struct *p, struct task_struct *parent);
extern void force_sig(int, struct task_struct *);
extern int send_sig(int, struct task_struct *, int);
-extern int zap_other_threads(struct task_struct *p);
+extern int zap_other_threads(struct task_struct *p, int do_count);
extern struct sigqueue *sigqueue_alloc(void);
extern void sigqueue_free(struct sigqueue *);
extern int send_sigqueue(struct sigqueue *, struct task_struct *, int group);
diff --git a/kernel/exit.c b/kernel/exit.c
index 8c5b3e106298..972df5ebf79f 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -712,6 +712,8 @@ static void forget_original_parent(struct task_struct *father,
*/
static void exit_notify(struct task_struct *tsk, int group_dead)
{
+ struct sighand_struct *sighand = tsk->sighand;
+ struct signal_struct *signal = tsk->signal;
bool autoreap;
struct task_struct *p, *n;
LIST_HEAD(dead);
@@ -739,9 +741,12 @@ static void exit_notify(struct task_struct *tsk, int group_dead)
if (tsk->exit_state == EXIT_DEAD)
list_add(&tsk->ptrace_entry, &dead);
- /* mt-exec, de_thread() is waiting for group leader */
- if (unlikely(tsk->signal->notify_count < 0))
- wake_up_process(tsk->signal->group_exit_task);
+ spin_lock(&sighand->siglock);
+ /* mt-exec, de_thread is waiting for threads to exit */
+ if (signal->notify_count < 0 && !++signal->notify_count)
+ wake_up_process(signal->group_exit_task);
+
+ spin_unlock(&sighand->siglock);
write_unlock_irq(&tasklist_lock);
list_for_each_entry_safe(p, n, &dead, ptrace_entry) {
@@ -975,7 +980,7 @@ do_group_exit(int exit_code)
else {
sig->group_exit_code = exit_code;
sig->flags = SIGNAL_GROUP_EXIT;
- zap_other_threads(current);
+ zap_other_threads(current, 0);
}
spin_unlock_irq(&sighand->siglock);
}
diff --git a/kernel/signal.c b/kernel/signal.c
index 986ef55641ea..e3a5bc239345 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1196,7 +1196,7 @@ force_sig_info(int sig, struct siginfo *info, struct task_struct *t)
/*
* Nuke all other threads in the group.
*/
-int zap_other_threads(struct task_struct *p)
+int zap_other_threads(struct task_struct *p, int do_count)
{
struct task_struct *t = p;
int count = 0;
@@ -1205,13 +1205,17 @@ int zap_other_threads(struct task_struct *p)
while_each_thread(p, t) {
task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK);
- count++;
+ if (do_count > 0)
+ count++;
/* Don't bother with already dead threads */
if (t->exit_state)
continue;
sigaddset(&t->pending.signal, SIGKILL);
signal_wake_up(t, 1);
+
+ if (do_count < 0)
+ count--;
}
return count;
--
2.10.1
Powered by blists - more mailing lists