All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <christian@brauner.io>
To: linux-kernel@vger.kernel.org, oleg@redhat.com
Cc: arnd@arndb.de, ebiederm@xmission.com, keescook@chromium.org,
	joel@joelfernandes.org, tglx@linutronix.de, tj@kernel.org,
	dhowells@redhat.com, jannh@google.com, luto@kernel.org,
	akpm@linux-foundation.org, cyphar@cyphar.com,
	torvalds@linux-foundation.org, viro@zeniv.linux.org.uk,
	kernel-team@android.com, Christian Brauner <christian@brauner.io>
Subject: [RFC][PATCH 1/5] exit: kill struct waitid_info
Date: Wed, 24 Jul 2019 16:46:47 +0200	[thread overview]
Message-ID: <20190724144651.28272-2-christian@brauner.io> (raw)
In-Reply-To: <20190724144651.28272-1-christian@brauner.io>

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@brauner.io>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Joel Fernandes (Google) <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Jann Horn <jannh@google.com>
Cc: Andy Lutomirsky <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
---
 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


  reply	other threads:[~2019-07-24 14:47 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-24 14:46 [PATCH 0/5] pidfd: waiting on processes through pidfds Christian Brauner
2019-07-24 14:46 ` Christian Brauner [this message]
2019-07-24 17:37   ` [RFC][PATCH 1/5] exit: kill struct waitid_info Linus Torvalds
2019-07-24 22:01     ` Christian Brauner
2019-07-25 12:46     ` Eric W. Biederman
2019-07-26  8:01       ` Christian Brauner
2019-07-26 11:59         ` Eric W. Biederman
2019-07-26 12:37           ` Christian Brauner
2019-07-25  9:40   ` Oleg Nesterov
2019-07-25 10:07     ` Christian Brauner
2019-07-24 14:46 ` [PATCH 2/5] pidfd: add pidfd_wait() Christian Brauner
2019-07-24 17:45   ` Linus Torvalds
2019-07-24 17:50     ` Christian Brauner
2019-07-24 17:52       ` Christian Brauner
2019-07-25 14:34     ` Eric W. Biederman
2019-07-25 10:16   ` Oleg Nesterov
2019-07-25 10:21     ` Christian Brauner
2019-07-26  8:19   ` Arnd Bergmann
2019-07-26  8:24     ` Christian Brauner
2019-07-26  9:57       ` Arnd Bergmann
2019-07-24 14:46 ` [PATCH 3/5] arch: wire-up pidfd_wait() Christian Brauner
2019-07-24 14:46   ` Christian Brauner
2019-07-24 14:46   ` Christian Brauner
2019-07-24 14:46   ` Christian Brauner
2019-07-24 14:46 ` [PATCH 4/5] pidfd: add CLONE_WAIT_PID Christian Brauner
2019-07-24 18:14   ` Jann Horn
2019-07-24 18:27     ` Christian Brauner
2019-07-24 19:07       ` Jann Horn
2019-07-24 19:10         ` Christian Brauner
2019-07-25 10:11           ` Christian Brauner
2019-07-25 10:30         ` Oleg Nesterov
2019-07-25 10:36           ` Christian Brauner
2019-07-25 10:35   ` Oleg Nesterov
2019-07-25 10:40     ` Christian Brauner
2019-07-25 11:25       ` Oleg Nesterov
2019-07-25 11:41         ` Christian Brauner
2019-07-25 11:43         ` Oleg Nesterov
2019-07-25 12:26           ` Christian Brauner
2019-07-25 16:13             ` Oleg Nesterov
2019-07-25 16:56               ` Christian Brauner
2019-07-25 16:57             ` Eric W. Biederman
2019-07-24 14:46 ` [PATCH 5/5] pidfd: add pidfd_wait tests Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190724144651.28272-2-christian@brauner.io \
    --to=christian@brauner.io \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=cyphar@cyphar.com \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=jannh@google.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=oleg@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.