These patches applied on top of Oleg's work in -mm (but don't interfere
with it):
do_wait-wakeup-optimization-shift-security_task_wait-from-eligible_child-to-wait_consider_task.patch
do_wait-wakeup-optimization-change-__wake_up_parent-to-use-filtered-wakeup.patch
do_wait-wakeup-optimization-child_wait_callback-check-__wnothread-case.patch
do_wait-optimization-do-not-place-sub-threads-on-task_struct-children-list.patch
wait_consider_task-kill-parent-argument.patch
Vitaly Mayatskikh (2):
do_wait: fix sys_waitid()-specific behaviour
Check for ->wo_info != NULL in wait_noreap_copyout()
kernel/exit.c | 75 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 37 insertions(+), 38 deletions(-)
Current behaviour of sys_waitid() looks odd. If user passes infop == NULL,
sys_waitid() returns success. When user additionally specifies flag WNOWAIT,
sys_waitid() returns -EFAULT on the same conditions. When user combines
WNOWAIT with WCONTINUED, sys_waitid() again returns success.
This patch adds check for ->wo_info in wait_noreap_copyout().
User-visible change: starting from this commit, sys_waitid() always checks
infop != NULL and does not fail if it is NULL.
Signed-off-by: Vitaly Mayatskikh <[email protected]>
Reviewed-by: Oleg Nesterov <[email protected]>
---
kernel/exit.c | 26 ++++++++++++++------------
1 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 62e3646..f306f20 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1134,18 +1134,20 @@ static int wait_noreap_copyout(struct wait_opts *wo, struct task_struct *p,
put_task_struct(p);
infop = wo->wo_info;
- if (!retval)
- retval = put_user(SIGCHLD, &infop->si_signo);
- if (!retval)
- retval = put_user(0, &infop->si_errno);
- if (!retval)
- retval = put_user((short)why, &infop->si_code);
- if (!retval)
- retval = put_user(pid, &infop->si_pid);
- if (!retval)
- retval = put_user(uid, &infop->si_uid);
- if (!retval)
- retval = put_user(status, &infop->si_status);
+ if (infop) {
+ if (!retval)
+ retval = put_user(SIGCHLD, &infop->si_signo);
+ if (!retval)
+ retval = put_user(0, &infop->si_errno);
+ if (!retval)
+ retval = put_user((short)why, &infop->si_code);
+ if (!retval)
+ retval = put_user(pid, &infop->si_pid);
+ if (!retval)
+ retval = put_user(uid, &infop->si_uid);
+ if (!retval)
+ retval = put_user(status, &infop->si_status);
+ }
if (!retval)
retval = pid;
return retval;
--
1.6.2.5
do_wait() checks ->wo_info to figure out who is the caller.
If it's not NULL the caller should be sys_waitid(), in that case
do_wait() fixes up the retval or zeros ->wo_info, depending on
retval from underlying function.
This is bug: user can pass ->wo_info == NULL and sys_waitid() will
return incorrect value.
>From man 2 waitid:
waitid(): returns 0 on success
Test-case:
int main(void)
{
if (fork())
assert(waitid(P_ALL, 0, NULL, WEXITED) == 0);
return 0;
}
Result:
Assertion `waitid(P_ALL, 0, ((void *)0), 4) == 0' failed.
Move that code to sys_waitid().
User-visible change: sys_waitid() will return 0 on success, either
infop is set or not.
Note, there's another bug in wait_noreap_copyout() which affects
return value of sys_waitid(). It will be fixed in next patch.
Signed-off-by: Vitaly Mayatskikh <[email protected]>
Reviewed-by: Oleg Nesterov <[email protected]>
---
kernel/exit.c | 49 +++++++++++++++++++++++--------------------------
1 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 80a15b6..62e3646 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1630,32 +1630,6 @@ notask:
end:
__set_current_state(TASK_RUNNING);
remove_wait_queue(¤t->signal->wait_chldexit, &wo->child_wait);
-
- if (wo->wo_info) {
- struct siginfo __user *infop = wo->wo_info;
-
- if (retval > 0)
- retval = 0;
- else {
- /*
- * For a WNOHANG return, clear out all the fields
- * we would set so the user can easily tell the
- * difference.
- */
- if (!retval)
- retval = put_user(0, &infop->si_signo);
- if (!retval)
- retval = put_user(0, &infop->si_errno);
- if (!retval)
- retval = put_user(0, &infop->si_code);
- if (!retval)
- retval = put_user(0, &infop->si_pid);
- if (!retval)
- retval = put_user(0, &infop->si_uid);
- if (!retval)
- retval = put_user(0, &infop->si_status);
- }
- }
return retval;
}
@@ -1700,6 +1674,29 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
wo.wo_stat = NULL;
wo.wo_rusage = ru;
ret = do_wait(&wo);
+
+ if (ret > 0) {
+ ret = 0;
+ } else if (infop) {
+ /*
+ * For a WNOHANG return, clear out all the fields
+ * we would set so the user can easily tell the
+ * difference.
+ */
+ if (!ret)
+ ret = put_user(0, &infop->si_signo);
+ if (!ret)
+ ret = put_user(0, &infop->si_errno);
+ if (!ret)
+ ret = put_user(0, &infop->si_code);
+ if (!ret)
+ ret = put_user(0, &infop->si_pid);
+ if (!ret)
+ ret = put_user(0, &infop->si_uid);
+ if (!ret)
+ ret = put_user(0, &infop->si_status);
+ }
+
put_pid(pid);
/* avoid REGPARM breakage on x86: */
--
1.6.2.5