Gitweb:     http://git.kernel.org/linus/4b105cbbaf7c06e01c27391957dc3c446328d087
Commit:     4b105cbbaf7c06e01c27391957dc3c446328d087
Parent:     f2f0b00ad61d53adfecb8bdf8f3cf8f05f6ed548
Author:     Oleg Nesterov <oleg@redhat.com>
AuthorDate: Wed Jun 17 16:27:33 2009 -0700
Committer:  Linus Torvalds <torvalds@linux-foundation.org>
CommitDate: Thu Jun 18 13:03:51 2009 -0700

    ptrace: do not use task_lock() for attach
    
    Remove the "Nasty, nasty" lock dance in ptrace_attach()/ptrace_traceme() -
    from now task_lock() has nothing to do with ptrace at all.
    
    With the recent changes nobody uses task_lock() to serialize with ptrace,
    but in fact it was never needed and it was never used consistently.
    
    However ptrace_attach() calls __ptrace_may_access() and needs task_lock()
    to pin task->mm for get_dumpable().  But we can call __ptrace_may_access()
    before we take tasklist_lock, ->cred_exec_mutex protects us against
    do_execve() path which can change creds and MMF_DUMP* flags.
    
    (ugly, but we can't use ptrace_may_access() because it hides the error
    code, so we have to take task_lock() and use __ptrace_may_access()).
    
    NOTE: this change assumes that LSM hooks, security_ptrace_may_access() and
    security_ptrace_traceme(), can be called without task_lock() held.
    
    Signed-off-by: Oleg Nesterov <oleg@redhat.com>
    Cc: Chris Wright <chrisw@sous-sol.org>
    Acked-by: Roland McGrath <roland@redhat.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 kernel/ptrace.c |   59 ++++++++++++------------------------------------------
 1 files changed, 13 insertions(+), 46 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 12e21a9..38fdfea 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -167,7 +167,6 @@ bool ptrace_may_access(struct task_struct *task, unsigned int mode)
 int ptrace_attach(struct task_struct *task)
 {
 	int retval;
-	unsigned long flags;
 
 	audit_ptrace(task);
 
@@ -185,34 +184,19 @@ int ptrace_attach(struct task_struct *task)
 	retval = mutex_lock_interruptible(&task->cred_guard_mutex);
 	if (retval < 0)
 		goto out;
-repeat:
-	/*
-	 * Nasty, nasty.
-	 *
-	 * We want to hold both the task-lock and the
-	 * tasklist_lock for writing at the same time.
-	 * But that's against the rules (tasklist_lock
-	 * is taken for reading by interrupts on other
-	 * cpu's that may have task_lock).
-	 */
-	task_lock(task);
-	if (!write_trylock_irqsave(&tasklist_lock, flags)) {
-		task_unlock(task);
-		do {
-			cpu_relax();
-		} while (!write_can_lock(&tasklist_lock));
-		goto repeat;
-	}
 
+	task_lock(task);
 	retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH);
+	task_unlock(task);
 	if (retval)
-		goto bad;
+		goto unlock_creds;
 
+	write_lock_irq(&tasklist_lock);
 	retval = -EPERM;
 	if (unlikely(task->exit_state))
-		goto bad;
+		goto unlock_tasklist;
 	if (task->ptrace)
-		goto bad;
+		goto unlock_tasklist;
 
 	task->ptrace = PT_PTRACED;
 	if (capable(CAP_SYS_PTRACE))
@@ -222,9 +206,9 @@ repeat:
 	send_sig_info(SIGSTOP, SEND_SIG_FORCED, task);
 
 	retval = 0;
-bad:
-	write_unlock_irqrestore(&tasklist_lock, flags);
-	task_unlock(task);
+unlock_tasklist:
+	write_unlock_irq(&tasklist_lock);
+unlock_creds:
 	mutex_unlock(&task->cred_guard_mutex);
 out:
 	return retval;
@@ -240,26 +224,10 @@ int ptrace_traceme(void)
 {
 	int ret = -EPERM;
 
-	/*
-	 * Are we already being traced?
-	 */
-repeat:
-	task_lock(current);
+	write_lock_irq(&tasklist_lock);
+	/* Are we already being traced? */
 	if (!current->ptrace) {
-		/*
-		 * See ptrace_attach() comments about the locking here.
-		 */
-		unsigned long flags;
-		if (!write_trylock_irqsave(&tasklist_lock, flags)) {
-			task_unlock(current);
-			do {
-				cpu_relax();
-			} while (!write_can_lock(&tasklist_lock));
-			goto repeat;
-		}
-
 		ret = security_ptrace_traceme(current->parent);
-
 		/*
 		 * Check PF_EXITING to ensure ->real_parent has not passed
 		 * exit_ptrace(). Otherwise we don't report the error but
@@ -269,10 +237,9 @@ repeat:
 			current->ptrace = PT_PTRACED;
 			__ptrace_link(current, current->real_parent);
 		}
-
-		write_unlock_irqrestore(&tasklist_lock, flags);
 	}
-	task_unlock(current);
+	write_unlock_irq(&tasklist_lock);
+
 	return ret;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html