[PATCH 11/14] signal: Use the thread killing in get_signal for coredumps

["Eric W. Biederman" <ebiederm@xmission.com>]

Now that coredumps are per process there is no reason for the coredump
code to have it's own routine to kill the threads of the process.  The
coredump code does need to have a routine to catch the threads that
will be part of the coredump, and to only catch them if a coredump
will be generated.

Split out coredump_begin from do_coredump so that the threads of the
process can be caught in the coredump.  Also move the logic to decide
if a coredump should be generated into coredump_begin, with
do_coredump now simply returning immediately if coredump_begin
has decided not to capture a coredump.

Update get_signal to always shoot down the threads of the process, and
to call coredump_begin if a coredump needs to be started.
Remove the call of do_group_exit in get_signal as it is unnecessary.

The practical reason for splitting coredump_begin out from
do_coredump is so that I don't have to analyze if cgroup_leave_frozen,
print_fatal_signal, proc_coredump_connector and audit_core_dumps can
safely be called under siglock.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/coredump.c                | 106 +++++++++++++++--------------------
 include/linux/coredump.h     |   2 +
 include/linux/sched/signal.h |   1 +
 kernel/signal.c              |  44 ++++++---------
 4 files changed, 64 insertions(+), 89 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 4e0e9407704c..998800f171a4 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -480,46 +480,50 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
 	return true;
 }
 
-static int zap_process(struct signal_struct *signal, int exit_code)
+static inline bool coredump_skip(enum task_dumpable dumpable,
+				 const struct linux_binfmt *binfmt)
 {
+	if (!binfmt)
+		return true;
+	if (!binfmt->core_dump)
+		return true;
+	if (dumpable == TASK_DUMPABLE_OFF)
+		return true;
+	return false;
+}
+
+void coredump_begin(struct core_state *core_state)
+{
+	/* Called with siglock held */
+	struct task_struct *tsk = current;
+	struct signal_struct *signal = tsk->signal;
+	struct mm_struct *mm = tsk->mm;
+	struct linux_binfmt * binfmt = mm->binfmt;
+	/* Snapshot dumpable for the dump */
+	enum task_dumpable dumpable = task_exec_state_get_dumpable(tsk);
 	struct task_struct *t;
 	int nr = 0;
 
-	signal->flags = SIGNAL_GROUP_EXIT;
-	signal->group_exit_code = exit_code;
-	signal->group_stop_count = 0;
+	if (coredump_skip(dumpable, binfmt))
+		return;
 
-	__for_each_thread(signal, t) {
-		task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK);
-		if (t != current && !(t->flags & PF_POSTCOREDUMP)) {
-			sigaddset(&t->pending.signal, SIGKILL);
-			signal_wake_up(t, 1);
-			nr++;
-		}
-	}
+	init_completion(&core_state->startup);
+	core_state->dumper.task = tsk;
+	core_state->dumper.next = NULL;
+	core_state->dumpable = dumpable;
 
-	return nr;
-}
+	/* Count how may other threads will participate in the coredump */
+	__for_each_thread(signal, t)
+		nr += (t != tsk) && !(t->flags & PF_POSTCOREDUMP);
 
-static int zap_threads(struct task_struct *tsk,
-			struct core_state *core_state, int exit_code)
-{
-	struct signal_struct *signal = tsk->signal;
-	int nr = -EAGAIN;
-
-	spin_lock_irq(&tsk->sighand->siglock);
-	if (!(signal->flags & SIGNAL_GROUP_EXIT) && !signal->group_exec_task) {
-		/* Allow SIGKILL, see prepare_signal() */
-		signal->core_state = core_state;
-		nr = zap_process(signal, exit_code);
-		clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
-		tsk->flags |= PF_DUMPCORE;
-		atomic_set(&core_state->nr_threads, nr);
-	}
-	if (nr <= 0)
+	atomic_set(&core_state->nr_threads, nr);
+	if (nr == 0)
 		complete(&core_state->startup);
-	spin_unlock_irq(&tsk->sighand->siglock);
-	return nr;
+
+	/* Allow SIGKILL, see prepare_signal() */
+	signal->core_state = core_state;
+	clear_tsk_thread_flag(tsk, TIF_SIGPENDING);
+	tsk->flags |= PF_DUMPCORE;
 }
 
 void coredump_join(struct core_state *core_state)
@@ -545,17 +549,9 @@ void coredump_join(struct core_state *core_state)
 	__set_current_state(TASK_RUNNING);
 }
 
-static int coredump_wait(int exit_code, struct core_state *core_state)
+static void coredump_wait(struct core_state *core_state)
 {
-	struct task_struct *tsk = current;
 	struct core_thread *ptr;
-	int core_waiters;
-
-	init_completion(&core_state->startup);
-	core_state->dumper.task = tsk;
-	core_state->dumper.next = NULL;
-
-	core_waiters = zap_threads(tsk, core_state, exit_code);
 
 	wait_for_completion_state(&core_state->startup,
 				  TASK_UNINTERRUPTIBLE|TASK_FREEZABLE);
@@ -569,8 +565,6 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 		wait_task_inactive(ptr->task, TASK_ANY);
 		ptr = ptr->next;
 	}
-
-	return core_waiters;
 }
 
 static void coredump_finish(bool core_dumped)
@@ -1100,18 +1094,6 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
 	coredump_finish(cn->core_dumped);
 }
 
-static inline bool coredump_skip(const struct coredump_params *cprm,
-				 const struct linux_binfmt *binfmt)
-{
-	if (!binfmt)
-		return true;
-	if (!binfmt->core_dump)
-		return true;
-	if (cprm->dumpable == TASK_DUMPABLE_OFF)
-		return true;
-	return false;
-}
-
 static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 			size_t **argv, int *argc, const struct linux_binfmt *binfmt)
 {
@@ -1184,7 +1166,7 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 void vfs_coredump(const kernel_siginfo_t *siginfo)
 {
 	size_t *argv __free(kfree) = NULL;
-	struct core_state core_state;
+	struct core_state *core_state = current->signal->core_state;
 	struct core_name cn;
 	const struct mm_struct *mm = current->mm;
 	const struct linux_binfmt *binfmt = mm->binfmt;
@@ -1192,16 +1174,19 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
 	struct coredump_params cprm = {
 		.siginfo = siginfo,
 		.limit = rlimit(RLIMIT_CORE),
-		/* Snapshot MMF_DUMP_FILTER_* (unlocked) and dumpable for the dump. */
+		/* Snapshot MMF_DUMP_FILTER_* (unlocked) for the dump */
 		.mm_flags = __mm_flags_get_word(mm),
-		.dumpable = task_exec_state_get_dumpable(current),
 		.vma_meta = NULL,
 		.cpu = raw_smp_processor_id(),
 	};
 
-	if (coredump_skip(&cprm, binfmt))
+	/* coredump_begin decided not to coredump */
+	if (!core_state)
 		return;
 
+	/* Copy the snapshot of dumpable into coredump_params */
+	cprm.dumpable = core_state->dumpable;
+
 	CLASS(prepare_creds, cred)();
 	if (!cred)
 		return;
@@ -1214,8 +1199,7 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
 	if (coredump_force_suid_safe(&cprm))
 		cred->fsuid = GLOBAL_ROOT_UID;
 
-	if (coredump_wait(siginfo->si_signo, &core_state) < 0)
-		return;
+	coredump_wait(core_state);
 
 	scoped_with_creds(cred)
 		do_coredump(&cn, &cprm, &argv, &argc, binfmt);
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 22f46392b4d3..645ea675dc91 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -47,6 +47,7 @@ extern int dump_emit(struct coredump_params *cprm, const void *addr, int nr);
 extern int dump_align(struct coredump_params *cprm, int align);
 int dump_user_range(struct coredump_params *cprm, unsigned long start,
 		    unsigned long len);
+extern void coredump_begin(struct core_state *core_state);
 extern void coredump_join(struct core_state *core_state);
 extern void vfs_coredump(const kernel_siginfo_t *siginfo);
 
@@ -68,6 +69,7 @@ extern void vfs_coredump(const kernel_siginfo_t *siginfo);
 #define coredump_report_failure(fmt, ...) __COREDUMP_PRINTK(KERN_WARNING, fmt, ##__VA_ARGS__)
 
 #else
+static inline void coredump_begin(struct core_state *core_state) {}
 extern inline void coredump_join(struct core_state *core_state) {}
 static inline void vfs_coredump(const kernel_siginfo_t *siginfo) {}
 
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 584ae88b435e..4ff1da6b841e 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -80,6 +80,7 @@ struct core_thread {
 
 struct core_state {
 	atomic_t nr_threads;
+	enum task_dumpable dumpable;
 	struct core_thread dumper;
 	struct completion startup;
 };
diff --git a/kernel/signal.c b/kernel/signal.c
index 28e047d76043..674d4b6d0b8a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2906,8 +2906,8 @@ bool get_signal(struct ksignal *ksig)
 	}
 
 	for (;;) {
-		bool group_exit_needed = false;
-		struct core_state *core_state;
+		struct core_state local_core_state, *core_state;
+		struct task_struct *t;
 		struct k_sigaction *ka;
 		enum pid_type type;
 		int exit_code = 0;
@@ -3050,22 +3050,20 @@ bool get_signal(struct ksignal *ksig)
 		 * Anything else is fatal, maybe with a core dump.
 		 */
 		exit_code = signr;
-		if (sig_kernel_coredump(signr))
-			group_exit_needed = true;
-		else {
-			struct task_struct *t;
-			signal->flags = SIGNAL_GROUP_EXIT;
-			signal->group_exit_code = signr;
-			signal->group_stop_count = 0;
-			__for_each_thread(signal, t) {
-				task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK);
-				if (t != current) {
-					sigaddset(&t->pending.signal, SIGKILL);
-					signal_wake_up(t, 1);
-				}
+		signal->flags = SIGNAL_GROUP_EXIT;
+		signal->group_exit_code = exit_code;
+		signal->group_stop_count = 0;
+		__for_each_thread(signal, t) {
+			task_clear_jobctl_pending(t, JOBCTL_PENDING_MASK);
+			if (t != current) {
+				sigaddset(&t->pending.signal, SIGKILL);
+				signal_wake_up(t, 1);
 			}
 		}
 	fatal:
+		/* Setup to collect a coredump */
+		if (sig_kernel_coredump(signr))
+			coredump_begin(&local_core_state);
 		core_state = signal->core_state;
 		spin_unlock_irq(&sighand->siglock);
 		if (unlikely(cgroup_task_frozen(current)))
@@ -3078,14 +3076,7 @@ bool get_signal(struct ksignal *ksig)
 				print_fatal_signal(signr);
 			proc_coredump_connector(current);
 			audit_core_dumps(ksig->info.si_signo);
-			/*
-			 * If it was able to dump core, this kills all
-			 * other threads in the group and synchronizes with
-			 * their demise.  If we lost the race with another
-			 * thread getting here, it set group_exit_code
-			 * first and our do_group_exit call below will use
-			 * that value and ignore the one we pass it.
-			 */
+			/* If dumping write out the coredump */
 			vfs_coredump(&ksig->info);
 		} else if (core_state) {
 			/* Wait for the coredump to happen */
@@ -3102,12 +3093,9 @@ bool get_signal(struct ksignal *ksig)
 			goto out;
 
 		/*
-		 * Death signals, no core dump.
+		 * Death signals.
 		 */
-		if (group_exit_needed)
-			do_group_exit(exit_code);
-		else
-			do_exit(exit_code);
+		do_exit(exit_code);
 		/* NOTREACHED */
 	}
 	spin_unlock_irq(&sighand->siglock);
-- 
2.41.0