Re: [PATCH 32/32] aio: implement io_pgetevents

[Christoph Hellwig <hch@lst.de>]

On Tue, Jan 16, 2018 at 07:41:24PM -0500, Jeff Moyer wrote:
> I'd be willing to bet the issue is in your io_syscall6 implementation.
> You pass in arg5 where arg6 should be used.  Don't feel bad, it took me
> the better part of today to figure that out.  :)
> 
> Here's an incremental diff on top of what you've posted.  Feel free to
> fold it into your patch (and format however you like).  You can find the
> libaio changes in my 'aio-poll' branch:
>   https://pagure.io/libaio/commits/aio-poll
> 
> These changes were run through the libaio test harness, 64 bit and 32
> bit, so the compat system call was tested.

Oops, yes.  Although I prefer the copy_from_user version, this is what
I had:


diff --git a/fs/aio.c b/fs/aio.c
index 9fe0a5539596..6c1bbfa9b06a 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1984,8 +1984,9 @@ SYSCALL_DEFINE6(io_pgetevents,
 		long, nr,
 		struct io_event __user *, events,
 		struct timespec __user *, timeout,
-		const sigset_t __user *, sigmask)
+		const struct __aio_sigset __user *, usig)
 {
+	struct __aio_sigset	ksig = { NULL, };
 	sigset_t		ksigmask, sigsaved;
 	struct timespec64	ts;
 	int ret;
@@ -1993,8 +1994,13 @@ SYSCALL_DEFINE6(io_pgetevents,
 	if (timeout && unlikely(get_timespec64(&ts, timeout)))
 		return -EFAULT;
 
-	if (sigmask) {
-		if (copy_from_user(&ksigmask, sigmask, sizeof(ksigmask)))
+	if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
+		return -EFAULT;
+
+	if (ksig.sigmask) {
+		if (ksig.sigsetsize != sizeof(sigset_t))
+			return -EINVAL;
+		if (copy_from_user(&ksigmask, ksig.sigmask, sizeof(ksigmask)))
 			return -EFAULT;
 		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
 		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
@@ -2002,7 +2008,7 @@ SYSCALL_DEFINE6(io_pgetevents,
 
 	ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
 	if (signal_pending(current)) {
-		if (sigmask) {
+		if (ksig.sigmask) {
 			current->saved_sigmask = sigsaved;
 			set_restore_sigmask();
 		}
@@ -2010,7 +2016,7 @@ SYSCALL_DEFINE6(io_pgetevents,
 		if (!ret)
 			ret = -ERESTARTNOHAND;
 	} else {
-		if (sigmask)
+		if (ksig.sigmask)
 			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
 	}
 
@@ -2036,14 +2042,21 @@ COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
 	return ret;
 }
 
+
+struct __compat_aio_sigset {
+	compat_sigset_t __user	*sigmask;
+	compat_size_t		sigsetsize;
+};
+
 COMPAT_SYSCALL_DEFINE6(io_pgetevents,
 		compat_aio_context_t, ctx_id,
 		compat_long_t, min_nr,
 		compat_long_t, nr,
 		struct io_event __user *, events,
 		struct compat_timespec __user *, timeout,
-		const compat_sigset_t __user *, sigmask)
+		const struct __compat_aio_sigset __user *, usig)
 {
+	struct __compat_aio_sigset ksig = { NULL, };
 	sigset_t ksigmask, sigsaved;
 	struct timespec64 t;
 	int ret;
@@ -2051,8 +2064,13 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents,
 	if (timeout && compat_get_timespec64(&t, timeout))
 		return -EFAULT;
 
-	if (sigmask) {
-		if (get_compat_sigset(&ksigmask, sigmask))
+	if (usig && copy_from_user(&ksig, usig, sizeof(ksig)))
+		return -EFAULT;
+
+	if (ksig.sigmask) {
+		if (ksig.sigsetsize != sizeof(compat_sigset_t))
+			return -EINVAL;
+		if (get_compat_sigset(&ksigmask, ksig.sigmask))
 			return -EFAULT;
 		sigdelsetmask(&ksigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
 		sigprocmask(SIG_SETMASK, &ksigmask, &sigsaved);
@@ -2060,14 +2078,14 @@ COMPAT_SYSCALL_DEFINE6(io_pgetevents,
 
 	ret = do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
 	if (signal_pending(current)) {
-		if (sigmask) {
+		if (ksig.sigmask) {
 			current->saved_sigmask = sigsaved;
 			set_restore_sigmask();
 		}
 		if (!ret)
 			ret = -ERESTARTNOHAND;
 	} else {
-		if (sigmask)
+		if (ksig.sigmask)
 			sigprocmask(SIG_SETMASK, &sigsaved, NULL);
 	}
 
diff --git a/include/linux/compat.h b/include/linux/compat.h
index a4cda98073f1..6c04450e961f 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -205,6 +205,7 @@ extern int put_compat_rusage(const struct rusage *,
 			     struct compat_rusage __user *);
 
 struct compat_siginfo;
+struct __compat_aio_sigset;
 
 extern asmlinkage long compat_sys_waitid(int, compat_pid_t,
 		struct compat_siginfo __user *, int,
@@ -541,7 +542,7 @@ asmlinkage long compat_sys_io_pgetevents(compat_aio_context_t ctx_id,
 					compat_long_t nr,
 					struct io_event __user *events,
 					struct compat_timespec __user *timeout,
-					const compat_sigset_t __user *sigmask);
+					const struct __compat_aio_sigset __user *usig);
 asmlinkage long compat_sys_io_submit(compat_aio_context_t ctx_id, int nr,
 				     u32 __user *iocb);
 asmlinkage long compat_sys_mount(const char __user *dev_name,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3bc9a130f4a9..8515ec53c81b 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -544,7 +544,7 @@ asmlinkage long sys_io_pgetevents(aio_context_t ctx_id,
 				long nr,
 				struct io_event __user *events,
 				struct timespec __user *timeout,
-				const sigset_t __user *sigmask);
+				const struct __aio_sigset *sig);
 asmlinkage long sys_io_submit(aio_context_t, long,
 				struct iocb __user * __user *);
 asmlinkage long sys_io_cancel(aio_context_t ctx_id, struct iocb __user *iocb,
diff --git a/include/uapi/linux/aio_abi.h b/include/uapi/linux/aio_abi.h
index 28330105a4b6..ed0185945bb2 100644
--- a/include/uapi/linux/aio_abi.h
+++ b/include/uapi/linux/aio_abi.h
@@ -29,6 +29,7 @@
 
 #include <linux/types.h>
 #include <linux/fs.h>
+#include <linux/signal.h>
 #include <asm/byteorder.h>
 
 typedef __kernel_ulong_t aio_context_t;
@@ -106,5 +107,10 @@ struct iocb {
 #undef IFBIG
 #undef IFLITTLE
 
+struct __aio_sigset {
+	sigset_t __user	*sigmask;
+	size_t		sigsetsize;
+};
+
 #endif /* __LINUX__AIO_ABI_H */