diff mbox series

[v2,2/6] exec: Lock more defensively in exec

Message ID 87ftalla80.fsf_-_@x220.int.ebiederm.org (mailing list archive)
State New, archived
Headers show
Series exec: s/group_exit_task/group_exec_task/ for clarity | expand

Commit Message

Eric W. Biederman June 23, 2020, 9:54 p.m. UTC
When taking the task_list_lock in de_thread also take the siglock.  This
makes de_thread closer to fork the canonical place where these locks are
taken.

To complete the defensiveness always take siglock when clearing
group_exit_task and notify_count.

This gives now gives the guarantee that group_exit_task and notify_count
are now always changed under siglock.  As anything multi-threaded in exec
is a rare and slow path I don't think we care if we take an extra lock in
practice.

The practical reason for doing this is to enable setting signal->flags along
with group_exit_task so that the function signal_group_exit can be simplified.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/exec.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/fs/exec.c b/fs/exec.c
index e6e8a9a70327..33b5d9229c01 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1171,6 +1171,7 @@  static int de_thread(struct task_struct *tsk)
 		for (;;) {
 			cgroup_threadgroup_change_begin(tsk);
 			write_lock_irq(&tasklist_lock);
+			spin_lock(lock);
 			/*
 			 * Do this under tasklist_lock to ensure that
 			 * exit_notify() can't miss ->group_exit_task
@@ -1179,6 +1180,7 @@  static int de_thread(struct task_struct *tsk)
 			if (likely(leader->exit_state))
 				break;
 			__set_current_state(TASK_KILLABLE);
+			spin_unlock(lock);
 			write_unlock_irq(&tasklist_lock);
 			cgroup_threadgroup_change_end(tsk);
 			schedule();
@@ -1234,14 +1236,17 @@  static int de_thread(struct task_struct *tsk)
 		 */
 		if (unlikely(leader->ptrace))
 			__wake_up_parent(leader, leader->parent);
+		spin_unlock(lock);
 		write_unlock_irq(&tasklist_lock);
 		cgroup_threadgroup_change_end(tsk);
 
 		release_task(leader);
 	}
 
+	spin_lock_irq(lock);
 	sig->group_exit_task = NULL;
 	sig->notify_count = 0;
+	spin_unlock_irq(lock);
 
 no_thread_group:
 	/* we have changed execution domain */
@@ -1252,10 +1257,12 @@  static int de_thread(struct task_struct *tsk)
 
 killed:
 	/* protects against exit_notify() and __exit_signal() */
-	read_lock(&tasklist_lock);
+	read_lock_irq(&tasklist_lock);
+	spin_lock(lock);
 	sig->group_exit_task = NULL;
 	sig->notify_count = 0;
-	read_unlock(&tasklist_lock);
+	spin_unlock(lock);
+	read_unlock_irq(&tasklist_lock);
 	return -EAGAIN;
 }