[RFC][PATCH 1/5] exit: kill struct waitid_info

From: Christian Brauner
Date: Wed Jul 24 2019 - 10:47:34 EST


The code here uses a struct waitid_info to catch basic information about
process exit including the pid, uid, status, and signal that caused the
process to exit. This information is then stuffed into a struct siginfo
for the waitid() syscall. That seems like an odd thing to do. We can
just pass down a siginfo_t struct directly which let's us cleanup and
simplify the whole code quite a bit.
This patch also simplifies the following implementation of pidfd_wait().

Note that this changes how struct siginfo is filled in for users of
waitid. POSIX doesn't mandate anything else other than si_pid, si_uid,
si_code, and si_signo. So it seems up to the implementation. In case
anyone relies on the old behavior we can just revert but I highly doubt
that users fill in siginfo_t before a call to waitid and expect all
fields other then the ones mention above to be untouched.

Signed-off-by: Christian Brauner <christian@xxxxxxxxxx>
Cc: Arnd Bergmann <arnd@xxxxxxxx>
Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: David Howells <dhowells@xxxxxxxxxx>
Cc: Jann Horn <jannh@xxxxxxxxxx>
Cc: Andy Lutomirsky <luto@xxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Aleksa Sarai <cyphar@xxxxxxxxxx>
Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
kernel/exit.c | 101 ++++++++++++++++++--------------------------------
1 file changed, 36 insertions(+), 65 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index a75b6a7f458a..73392a455b72 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -994,19 +994,12 @@ SYSCALL_DEFINE1(exit_group, int, error_code)
return 0;
}

-struct waitid_info {
- pid_t pid;
- uid_t uid;
- int status;
- int cause;
-};
-
struct wait_opts {
enum pid_type wo_type;
int wo_flags;
struct pid *wo_pid;

- struct waitid_info *wo_info;
+ kernel_siginfo_t *wo_info;
int wo_stat;
struct rusage *wo_rusage;

@@ -1058,7 +1051,7 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
int state, status;
pid_t pid = task_pid_vnr(p);
uid_t uid = from_kuid_munged(current_user_ns(), task_uid(p));
- struct waitid_info *infop;
+ kernel_siginfo_t *infop;

if (!likely(wo->wo_flags & WEXITED))
return 0;
@@ -1169,14 +1162,14 @@ static int wait_task_zombie(struct wait_opts *wo, struct task_struct *p)
infop = wo->wo_info;
if (infop) {
if ((status & 0x7f) == 0) {
- infop->cause = CLD_EXITED;
- infop->status = status >> 8;
+ infop->si_code = CLD_EXITED;
+ infop->si_status = status >> 8;
} else {
- infop->cause = (status & 0x80) ? CLD_DUMPED : CLD_KILLED;
- infop->status = status & 0x7f;
+ infop->si_code = (status & 0x80) ? CLD_DUMPED : CLD_KILLED;
+ infop->si_status = status & 0x7f;
}
- infop->pid = pid;
- infop->uid = uid;
+ infop->si_pid = pid;
+ infop->si_uid = uid;
}

return pid;
@@ -1215,7 +1208,7 @@ static int *task_stopped_code(struct task_struct *p, bool ptrace)
static int wait_task_stopped(struct wait_opts *wo,
int ptrace, struct task_struct *p)
{
- struct waitid_info *infop;
+ kernel_siginfo_t *infop;
int exit_code, *p_code, why;
uid_t uid = 0; /* unneeded, required by compiler */
pid_t pid;
@@ -1270,10 +1263,10 @@ static int wait_task_stopped(struct wait_opts *wo,

infop = wo->wo_info;
if (infop) {
- infop->cause = why;
- infop->status = exit_code;
- infop->pid = pid;
- infop->uid = uid;
+ infop->si_code = why;
+ infop->si_status = exit_code;
+ infop->si_pid = pid;
+ infop->si_uid = uid;
}
return pid;
}
@@ -1286,7 +1279,7 @@ static int wait_task_stopped(struct wait_opts *wo,
*/
static int wait_task_continued(struct wait_opts *wo, struct task_struct *p)
{
- struct waitid_info *infop;
+ kernel_siginfo_t *infop;
pid_t pid;
uid_t uid;

@@ -1316,13 +1309,13 @@ static int wait_task_continued(struct wait_opts *wo, struct task_struct *p)
put_task_struct(p);

infop = wo->wo_info;
- if (!infop) {
- wo->wo_stat = 0xffff;
+ if (infop) {
+ infop->si_code = CLD_CONTINUED;
+ infop->si_status = SIGCONT;
+ infop->si_pid = pid;
+ infop->si_uid = uid;
} else {
- infop->cause = CLD_CONTINUED;
- infop->pid = pid;
- infop->uid = uid;
- infop->status = SIGCONT;
+ wo->wo_stat = 0xffff;
}
return pid;
}
@@ -1552,7 +1545,7 @@ static long do_wait(struct wait_opts *wo)
return retval;
}

-static long kernel_waitid(int which, pid_t upid, struct waitid_info *infop,
+static long kernel_waitid(int which, pid_t upid, kernel_siginfo_t *infop,
int options, struct rusage *ru)
{
struct wait_opts wo;
@@ -1602,33 +1595,22 @@ SYSCALL_DEFINE5(waitid, int, which, pid_t, upid, struct siginfo __user *,
infop, int, options, struct rusage __user *, ru)
{
struct rusage r;
- struct waitid_info info = {.status = 0};
- long err = kernel_waitid(which, upid, &info, options, ru ? &r : NULL);
- int signo = 0;
-
+ kernel_siginfo_t kinfo = {
+ .si_signo = 0,
+ };
+ long err = kernel_waitid(which, upid, infop ? &kinfo : NULL, options,
+ ru ? &r : NULL);
if (err > 0) {
- signo = SIGCHLD;
+ kinfo.si_signo = SIGCHLD;
err = 0;
if (ru && copy_to_user(ru, &r, sizeof(struct rusage)))
return -EFAULT;
}
- if (!infop)
- return err;

- if (!user_access_begin(infop, sizeof(*infop)))
+ if (infop && copy_siginfo_to_user(infop, &kinfo))
return -EFAULT;

- unsafe_put_user(signo, &infop->si_signo, Efault);
- unsafe_put_user(0, &infop->si_errno, Efault);
- unsafe_put_user(info.cause, &infop->si_code, Efault);
- unsafe_put_user(info.pid, &infop->si_pid, Efault);
- unsafe_put_user(info.uid, &infop->si_uid, Efault);
- unsafe_put_user(info.status, &infop->si_status, Efault);
- user_access_end();
- return err;
-Efault:
- user_access_end();
- return -EFAULT;
+ return err ;
}

long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
@@ -1722,11 +1704,13 @@ COMPAT_SYSCALL_DEFINE5(waitid,
struct compat_rusage __user *, uru)
{
struct rusage ru;
- struct waitid_info info = {.status = 0};
- long err = kernel_waitid(which, pid, &info, options, uru ? &ru : NULL);
- int signo = 0;
+ kernel_siginfo_t kinfo = {
+ .si_signo = 0,
+ };
+ long err = kernel_waitid(which, pid, infop ? &kinfo : NULL, options,
+ uru ? &ru : NULL);
if (err > 0) {
- signo = SIGCHLD;
+ kinfo.si_signo = SIGCHLD;
err = 0;
if (uru) {
/* kernel_waitid() overwrites everything in ru */
@@ -1739,23 +1723,10 @@ COMPAT_SYSCALL_DEFINE5(waitid,
}
}

- if (!infop)
- return err;
-
- if (!user_access_begin(infop, sizeof(*infop)))
+ if (infop && copy_siginfo_to_user32(infop, &kinfo))
return -EFAULT;

- unsafe_put_user(signo, &infop->si_signo, Efault);
- unsafe_put_user(0, &infop->si_errno, Efault);
- unsafe_put_user(info.cause, &infop->si_code, Efault);
- unsafe_put_user(info.pid, &infop->si_pid, Efault);
- unsafe_put_user(info.uid, &infop->si_uid, Efault);
- unsafe_put_user(info.status, &infop->si_status, Efault);
- user_access_end();
return err;
-Efault:
- user_access_end();
- return -EFAULT;
}
#endif

--
2.22.0