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

[Eric W. Biederman]

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@xxxxxxxxxxxx>
---
 fs/coredump.c                | 114 +++++++++++++++--------------------
 include/linux/coredump.h     |   2 +
 include/linux/sched/signal.h |   1 +
 kernel/signal.c              |  44 +++++---------
 4 files changed, 66 insertions(+), 95 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 0aa235429cfa..26bd1b3e9a03 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -481,46 +481,49 @@ 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(unsigned long mm_flags,
+				 const struct linux_binfmt *binfmt)
+{
+	if (!binfmt)
+		return true;
+	if (!binfmt->core_dump)
+		return true;
+	if (!__get_dumpable(mm_flags))
+		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;
+	unsigned long mm_flags = __mm_flags_get_dumpable(mm);
 	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(mm_flags, 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->mm_flags = mm_flags;
 
-	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)
@@ -546,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);
@@ -570,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)
@@ -1101,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 (!__get_dumpable(cprm->mm_flags))
-		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)
 {
@@ -1185,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;
@@ -1193,21 +1174,21 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
 	struct coredump_params cprm = {
 		.siginfo = siginfo,
 		.limit = rlimit(RLIMIT_CORE),
-		/*
-		 * We must use the same mm->flags while dumping core to avoid
-		 * inconsistency of bit flags, since this flag is not protected
-		 * by any locks.
-		 *
-		 * Note that we only care about MMF_DUMP* flags.
-		 */
-		.mm_flags = __mm_flags_get_dumpable(mm),
 		.vma_meta = NULL,
 		.cpu = raw_smp_processor_id(),
 	};
 
-	if (coredump_skip(&cprm, binfmt))
+	/* coredump_begin decided not to coredump */
+	if (!core_state)
 		return;
 
+	/*
+	 * We must use the same mm->flags while dumping core to avoid
+	 * inconsistency of bit flags, since this flag is not protected
+	 * by any locks.
+	 */
+	cprm.mm_flags = core_state->mm_flags;
+
 	CLASS(prepare_creds, cred)();
 	if (!cred)
 		return;
@@ -1220,8 +1201,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 c183c95f9063..d315ddccbf95 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -43,6 +43,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);
 
@@ -64,6 +65,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..1ea0a89cbef0 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;
+	unsigned long mm_flags;
 	struct core_thread dumper;
 	struct completion startup;
 };
diff --git a/kernel/signal.c b/kernel/signal.c
index 986221bb0e0a..89075c60b92b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2863,8 +2863,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;
@@ -3007,22 +3007,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)))
@@ -3035,14 +3033,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 */
@@ -3059,12 +3050,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