Gitweb:     
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f2cc3eb133baa2e9dc8efd40f417106b2ee520f3
Commit:     f2cc3eb133baa2e9dc8efd40f417106b2ee520f3
Parent:     96fabbf55ae79826f2e8a86f4066d7e8834315ae
Author:     Oleg Nesterov <[EMAIL PROTECTED]>
AuthorDate: Fri Feb 8 04:19:06 2008 -0800
Committer:  Linus Torvalds <[EMAIL PROTECTED]>
CommitDate: Fri Feb 8 09:22:26 2008 -0800

    do_wait: fix security checks
    
    Imho, the current usage of security_task_wait() is not logical.
    
    Suppose we have the single child p, and security_task_wait(p) return
    -EANY.  In that case waitpid(-1) returns this error.  Why? Isn't it
    better to return ECHLD? We don't really have reapable children.
    
    Now suppose that child was stolen by gdb.  In that case we find this
    child on ->ptrace_children and set flag = 1, but we don't check that the
    child was denied.  So, do_wait(..., WNOHANG) returns 0, this doesn't
    match the behaviour above.  Without WNOHANG do_wait() blocks only to
    return the error later, when the child will be untraced.  Inho, really
    strange.
    
    I think eligible_child() should return the error only if the child's pid
    was requested explicitly, otherwise we should silently ignore the tasks
    which were nacked by security_task_wait().
    
    Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]>
    Cc: Roland McGrath <[EMAIL PROTECTED]>
    Cc: Chris Wright <[EMAIL PROTECTED]>
    Cc: Eric Paris <[EMAIL PROTECTED]>
    Cc: James Morris <[EMAIL PROTECTED]>
    Cc: Stephen Smalley <[EMAIL PROTECTED]>
    Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
    Signed-off-by: Linus Torvalds <[EMAIL PROTECTED]>
---
 kernel/exit.c |   39 ++++++++++++++++++---------------------
 1 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 9ee229e..ee60772 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1142,10 +1142,14 @@ static int eligible_child(pid_t pid, int options, 
struct task_struct *p)
                return 0;
 
        err = security_task_wait(p);
-       if (err)
-               return err;
+       if (likely(!err))
+               return 1;
 
-       return 1;
+       if (pid <= 0)
+               return 0;
+       /* This child was explicitly requested, abort */
+       read_unlock(&tasklist_lock);
+       return err;
 }
 
 static int wait_noreap_copyout(struct task_struct *p, pid_t pid, uid_t uid,
@@ -1476,7 +1480,6 @@ static long do_wait(pid_t pid, int options, struct 
siginfo __user *infop,
        DECLARE_WAITQUEUE(wait, current);
        struct task_struct *tsk;
        int flag, retval;
-       int allowed, denied;
 
        add_wait_queue(&current->signal->wait_chldexit,&wait);
 repeat:
@@ -1484,8 +1487,7 @@ repeat:
         * We will set this flag if we see any child that might later
         * match our criteria, even if we are not able to reap it yet.
         */
-       flag = 0;
-       allowed = denied = 0;
+       flag = retval = 0;
        current->state = TASK_INTERRUPTIBLE;
        read_lock(&tasklist_lock);
        tsk = current;
@@ -1498,13 +1500,8 @@ repeat:
                                continue;
 
                        if (unlikely(ret < 0)) {
-                               denied = ret;
-                               continue;
-                       }
-                       allowed = 1;
-
-                       retval = 0;
-                       if (task_is_stopped_or_traced(p)) {
+                               retval = ret;
+                       } else if (task_is_stopped_or_traced(p)) {
                                /*
                                 * It's stopped now, so it might later
                                 * continue, exit, or stop again.
@@ -1544,11 +1541,14 @@ repeat:
                }
                if (!flag) {
                        list_for_each_entry(p, &tsk->ptrace_children,
-                                           ptrace_list) {
-                               if (!eligible_child(pid, options, p))
+                                                               ptrace_list) {
+                               flag = eligible_child(pid, options, p);
+                               if (!flag)
                                        continue;
-                               flag = 1;
-                               break;
+                               if (likely(flag > 0))
+                                       break;
+                               retval = flag;
+                               goto end;
                        }
                }
                if (options & __WNOTHREAD)
@@ -1556,10 +1556,9 @@ repeat:
                tsk = next_thread(tsk);
                BUG_ON(tsk->signal != current->signal);
        } while (tsk != current);
-
        read_unlock(&tasklist_lock);
+
        if (flag) {
-               retval = 0;
                if (options & WNOHANG)
                        goto end;
                retval = -ERESTARTSYS;
@@ -1569,8 +1568,6 @@ repeat:
                goto repeat;
        }
        retval = -ECHILD;
-       if (unlikely(denied) && !allowed)
-               retval = denied;
 end:
        current->state = TASK_RUNNING;
        remove_wait_queue(&current->signal->wait_chldexit,&wait);
-
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to