aboutsummaryrefslogtreecommitdiffstats
path: root/kernel
diff options
context:
space:
mode:
authorOleg Nesterov <oleg@tv-sign.ru>2008-02-08 07:19:06 -0500
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2008-02-08 12:22:26 -0500
commitf2cc3eb133baa2e9dc8efd40f417106b2ee520f3 (patch)
tree37b08158ee5296e79a61aad086be36c742825e3b /kernel
parent96fabbf55ae79826f2e8a86f4066d7e8834315ae (diff)
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 <oleg@tv-sign.ru> Cc: Roland McGrath <roland@redhat.com> Cc: Chris Wright <chrisw@sous-sol.org> Cc: Eric Paris <eparis@redhat.com> Cc: James Morris <jmorris@namei.org> Cc: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'kernel')
-rw-r--r--kernel/exit.c39
1 files changed, 18 insertions, 21 deletions
diff --git a/kernel/exit.c b/kernel/exit.c
index 9ee229ea97e4..ee607720ae58 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)
1142 return 0; 1142 return 0;
1143 1143
1144 err = security_task_wait(p); 1144 err = security_task_wait(p);
1145 if (err) 1145 if (likely(!err))
1146 return err; 1146 return 1;
1147 1147
1148 return 1; 1148 if (pid <= 0)
1149 return 0;
1150 /* This child was explicitly requested, abort */
1151 read_unlock(&tasklist_lock);
1152 return err;
1149} 1153}
1150 1154
1151static int wait_noreap_copyout(struct task_struct *p, pid_t pid, uid_t uid, 1155static 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,
1476 DECLARE_WAITQUEUE(wait, current); 1480 DECLARE_WAITQUEUE(wait, current);
1477 struct task_struct *tsk; 1481 struct task_struct *tsk;
1478 int flag, retval; 1482 int flag, retval;
1479 int allowed, denied;
1480 1483
1481 add_wait_queue(&current->signal->wait_chldexit,&wait); 1484 add_wait_queue(&current->signal->wait_chldexit,&wait);
1482repeat: 1485repeat:
@@ -1484,8 +1487,7 @@ repeat:
1484 * We will set this flag if we see any child that might later 1487 * We will set this flag if we see any child that might later
1485 * match our criteria, even if we are not able to reap it yet. 1488 * match our criteria, even if we are not able to reap it yet.
1486 */ 1489 */
1487 flag = 0; 1490 flag = retval = 0;
1488 allowed = denied = 0;
1489 current->state = TASK_INTERRUPTIBLE; 1491 current->state = TASK_INTERRUPTIBLE;
1490 read_lock(&tasklist_lock); 1492 read_lock(&tasklist_lock);
1491 tsk = current; 1493 tsk = current;
@@ -1498,13 +1500,8 @@ repeat:
1498 continue; 1500 continue;
1499 1501
1500 if (unlikely(ret < 0)) { 1502 if (unlikely(ret < 0)) {
1501 denied = ret; 1503 retval = ret;
1502 continue; 1504 } else if (task_is_stopped_or_traced(p)) {
1503 }
1504 allowed = 1;
1505
1506 retval = 0;
1507 if (task_is_stopped_or_traced(p)) {
1508 /* 1505 /*
1509 * It's stopped now, so it might later 1506 * It's stopped now, so it might later
1510 * continue, exit, or stop again. 1507 * continue, exit, or stop again.
@@ -1544,11 +1541,14 @@ repeat:
1544 } 1541 }
1545 if (!flag) { 1542 if (!flag) {
1546 list_for_each_entry(p, &tsk->ptrace_children, 1543 list_for_each_entry(p, &tsk->ptrace_children,
1547 ptrace_list) { 1544 ptrace_list) {
1548 if (!eligible_child(pid, options, p)) 1545 flag = eligible_child(pid, options, p);
1546 if (!flag)
1549 continue; 1547 continue;
1550 flag = 1; 1548 if (likely(flag > 0))
1551 break; 1549 break;
1550 retval = flag;
1551 goto end;
1552 } 1552 }
1553 } 1553 }
1554 if (options & __WNOTHREAD) 1554 if (options & __WNOTHREAD)
@@ -1556,10 +1556,9 @@ repeat:
1556 tsk = next_thread(tsk); 1556 tsk = next_thread(tsk);
1557 BUG_ON(tsk->signal != current->signal); 1557 BUG_ON(tsk->signal != current->signal);
1558 } while (tsk != current); 1558 } while (tsk != current);
1559
1560 read_unlock(&tasklist_lock); 1559 read_unlock(&tasklist_lock);
1560
1561 if (flag) { 1561 if (flag) {
1562 retval = 0;
1563 if (options & WNOHANG) 1562 if (options & WNOHANG)
1564 goto end; 1563 goto end;
1565 retval = -ERESTARTSYS; 1564 retval = -ERESTARTSYS;
@@ -1569,8 +1568,6 @@ repeat:
1569 goto repeat; 1568 goto repeat;
1570 } 1569 }
1571 retval = -ECHILD; 1570 retval = -ECHILD;
1572 if (unlikely(denied) && !allowed)
1573 retval = denied;
1574end: 1571end:
1575 current->state = TASK_RUNNING; 1572 current->state = TASK_RUNNING;
1576 remove_wait_queue(&current->signal->wait_chldexit,&wait); 1573 remove_wait_queue(&current->signal->wait_chldexit,&wait);