linux-kernel - [PATCH] aio: rewrite aio_thread_op_foo_at to avoid 64-bit get

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>] [day] [month] [year] [list]

Message-Id: <1458077950-500866-1-git-send-email-arnd@arndb.de>
Date:	Tue, 15 Mar 2016 22:38:51 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Benjamin LaHaise <bcrl@...ck.org>
Cc:	Arnd Bergmann <arnd@...db.de>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-aio@...ck.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] aio: rewrite aio_thread_op_foo_at to avoid 64-bit get_user

Most architectures cannot access 64-bit integers using get_user
or __get_user, so we get a build error in the new aio_thread_op_foo_at
function:

fs/built-in.o: In function `aio_thread_op_foo_at':
aio.c:(.text+0x252de): undefined reference to `__get_user_bad'
aio.c:(.text+0x252e4): undefined reference to `__get_user_bad'

This replaces the function with a different implementation using
copy_from_user() on the iocb, to avoid the problem. As there are
already three calls to get_user() in the function, this likely
ends up being more efficient in particular on architectures that
have strong memory protection between kernel and user space
and now only need to switch access modes once.

I've also tried to make the function more readable overall,
with local variable names matching the callback function
arguments in both type and name, rather than matching what
we get from user space.

Signed-off-by: Arnd Bergmann <arnd@...db.de>
Fixes: d2f7a973e11e ("aio: don't use __get_user() for 64 bit values")
---
 fs/aio.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 72a7e8a2f67e..1748fb97d991 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1777,25 +1777,23 @@ static long aio_do_unlinkat(int fd, const char *filename, int flags, int mode)
 
 static long aio_thread_op_foo_at(struct aio_kiocb *req)
 {
-	u64 buf, offset;
-	long ret;
-	u32 fd;
+	struct iocb iocb;
+	int fd;
+	const char __user *filename;
+	int flags, mode;
+	do_foo_at_t do_foo_at;
 
-	if (unlikely(get_user(fd, &req->ki_user_iocb->aio_fildes)))
-		ret = -EFAULT;
-	else if (unlikely(get_user(buf, &req->ki_user_iocb->aio_buf)))
-		ret = -EFAULT;
-	else if (unlikely(get_user(offset, &req->ki_user_iocb->aio_offset)))
-		ret = -EFAULT;
-	else {
-		do_foo_at_t do_foo_at = (void *)req->ki_data;
+	if (copy_from_user(&iocb, req->ki_user_iocb, sizeof(struct iocb)))
+		return -EFAULT;
 
-		ret = do_foo_at((s32)fd,
-				(const char __user *)(long)buf,
-				(int)offset,
-				(unsigned short)(offset >> 32));
-	}
-	return ret;
+	fd = (s32)iocb.aio_fildes;
+	filename = (const char __user *)(uintptr_t)iocb.aio_buf;
+	flags = (int)lower_32_bits(iocb.aio_offset);
+	mode = (unsigned short)upper_32_bits(iocb.aio_offset);
+
+	do_foo_at = (void *)req->ki_data;
+
+	return do_foo_at(fd, filename, flags, mode);
 }
 
 static void openat_destruct(struct aio_kiocb *req)
-- 
2.7.0