ptrace: cleanup check/set of PT_PTRACED during attach
authorOleg Nesterov <oleg@redhat.com>
Wed, 17 Jun 2009 23:27:32 +0000 (16:27 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 18 Jun 2009 20:03:51 +0000 (13:03 -0700)
ptrace_attach() and ptrace_traceme() are the last functions which look as
if the untraced task can have task->ptrace != 0, this must not be
possible.  Change the code to just check ->ptrace != 0 and s/|=/=/ to set
PT_PTRACED.

Also, a couple of trivial whitespace cleanups in ptrace_attach().

And move ptrace_traceme() up near ptrace_attach() to keep them close to
each other.

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

index 9439bd3331a66a5c541fe5c8ae02c6a70c482290..12e21a949db14645a886fa4ef69963f9e451ca9d 100644 (file)
@@ -177,12 +177,13 @@ int ptrace_attach(struct task_struct *task)
        if (same_thread_group(task, current))
                goto out;
 
-       /* Protect the target's credential calculations against our
+       /*
+        * Protect exec's credential calculations against our interference;
         * interference; SUID, SGID and LSM creds get determined differently
         * under ptrace.
         */
        retval = mutex_lock_interruptible(&task->cred_guard_mutex);
-       if (retval  < 0)
+       if (retval < 0)
                goto out;
 repeat:
        /*
@@ -210,10 +211,10 @@ repeat:
        retval = -EPERM;
        if (unlikely(task->exit_state))
                goto bad;
-       if (task->ptrace & PT_PTRACED)
+       if (task->ptrace)
                goto bad;
 
-       task->ptrace |= PT_PTRACED;
+       task->ptrace = PT_PTRACED;
        if (capable(CAP_SYS_PTRACE))
                task->ptrace |= PT_PTRACE_CAP;
 
@@ -229,6 +230,52 @@ out:
        return retval;
 }
 
+/**
+ * ptrace_traceme  --  helper for PTRACE_TRACEME
+ *
+ * Performs checks and sets PT_PTRACED.
+ * Should be used by all ptrace implementations for PTRACE_TRACEME.
+ */
+int ptrace_traceme(void)
+{
+       int ret = -EPERM;
+
+       /*
+        * Are we already being traced?
+        */
+repeat:
+       task_lock(current);
+       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
+                * pretend ->real_parent untraces us right after return.
+                */
+               if (!ret && !(current->real_parent->flags & PF_EXITING)) {
+                       current->ptrace = PT_PTRACED;
+                       __ptrace_link(current, current->real_parent);
+               }
+
+               write_unlock_irqrestore(&tasklist_lock, flags);
+       }
+       task_unlock(current);
+       return ret;
+}
+
 /*
  * Called with irqs disabled, returns true if childs should reap themselves.
  */
@@ -567,52 +614,6 @@ int ptrace_request(struct task_struct *child, long request,
        return ret;
 }
 
-/**
- * ptrace_traceme  --  helper for PTRACE_TRACEME
- *
- * Performs checks and sets PT_PTRACED.
- * Should be used by all ptrace implementations for PTRACE_TRACEME.
- */
-int ptrace_traceme(void)
-{
-       int ret = -EPERM;
-
-       /*
-        * Are we already being traced?
-        */
-repeat:
-       task_lock(current);
-       if (!(current->ptrace & PT_PTRACED)) {
-               /*
-                * 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
-                * pretend ->real_parent untraces us right after return.
-                */
-               if (!ret && !(current->real_parent->flags & PF_EXITING)) {
-                       current->ptrace |= PT_PTRACED;
-                       __ptrace_link(current, current->real_parent);
-               }
-
-               write_unlock_irqrestore(&tasklist_lock, flags);
-       }
-       task_unlock(current);
-       return ret;
-}
-
 /**
  * ptrace_get_task_struct  --  grab a task struct reference for ptrace
  * @pid:       process id to grab a task_struct reference of