lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 24 Jul 2019 16:46:47 +0200
From:   Christian Brauner <christian@...uner.io>
To:     linux-kernel@...r.kernel.org, oleg@...hat.com
Cc:     arnd@...db.de, ebiederm@...ssion.com, keescook@...omium.org,
        joel@...lfernandes.org, tglx@...utronix.de, tj@...nel.org,
        dhowells@...hat.com, jannh@...gle.com, luto@...nel.org,
        akpm@...ux-foundation.org, cyphar@...har.com,
        torvalds@...ux-foundation.org, viro@...iv.linux.org.uk,
        kernel-team@...roid.com, Christian Brauner <christian@...uner.io>
Subject: [RFC][PATCH 1/5] exit: kill struct waitid_info

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@...uner.io>
Cc: Arnd Bergmann <arnd@...db.de>
Cc: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Joel Fernandes (Google) <joel@...lfernandes.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Tejun Heo <tj@...nel.org>
Cc: David Howells <dhowells@...hat.com>
Cc: Jann Horn <jannh@...gle.com>
Cc: Andy Lutomirsky <luto@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Aleksa Sarai <cyphar@...har.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Al Viro <viro@...iv.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ