fs: select/pselect buffer overrun with x32 abi

Commit Message

Lance Richardson Feb. 27, 2018, 12:31 p.m. UTC

The definition of fd_set in X32 user-space uses a 32-bit base
data type for the fd array while the kernel uses a 64-bit base
data type. For applications using the glibc implementation of
select(2)/pselect(2), the size of fd_set is an integer multiple
of both base types, so there is no issue.

For applications using fd_set sizes that are different from
the glibc default size, an overrun of the user-space fd_set
buffer will occur when the user-space buffer size is an odd
multiple of 4 bytes (e.g. user-space can pass a 12-byte fd_set
to the kernel and the kernel will copy 16 bytes to user-space
before returning from select/pselect system calls). OpenSSH is
one example of an application using fd_set sizes different from
the default.

Reproducer (compiled with gcc -mx32):

 #include <stdlib.h>
 #include <stdio.h>
 #include <mcheck.h>
 #include <sys/select.h>

 #define howmany(x, y) (((x) + (y) - 1) / (y))

 int test(int nfds)
 {
     fd_set *fds;
     struct timeval tv = {0, 0};

     printf("Calling select with nfds = %d\n", nfds);
     fds = calloc(howmany(nfds, __NFDBITS), sizeof(__fd_mask));
     select(nfds, fds, NULL, NULL, &tv);
     free(fds);
     printf("Success\n");

     return 0;
 }

 int main(int argc, char **argv)
 {
     mcheck_pedantic(NULL);

     test(64);
     test(65);
     return 0;
 }

 /* Expected output without this patch:
  *
  * Calling select with nfds = 64
  * Success
  * Calling select with nfds = 65
  * memory clobbered past end of allocated block
  * Aborted (core dumped)
  */

Signed-off-by: Lance Richardson <[e-mail]>
---
 fs/select.c | 78 +++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 25 deletions(-)

Patch

diff --git a/fs/select.c b/fs/select.c
index b6c36254028a..280176fff5e8 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -384,6 +384,31 @@  void zero_fd_set(unsigned long nr, unsigned long *fdset)
 	memset(fdset, 0, FDS_BYTES(nr));
 }
 
+/*
+ * Ooo, nasty.  We need here to frob 32-bit unsigned longs to
+ * 64-bit unsigned longs.
+ */
+static
+int compat_get_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
+			unsigned long *fdset)
+{
+	if (ufdset) {
+		return compat_get_bitmap(fdset, ufdset, nr);
+	} else {
+		zero_fd_set(nr, fdset);
+		return 0;
+	}
+}
+
+static
+int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
+		      unsigned long *fdset)
+{
+	if (!ufdset)
+		return 0;
+	return compat_put_bitmap(ufdset, fdset, nr);
+}
+
 #define FDS_IN(fds, n)		(fds->in + n)
 #define FDS_OUT(fds, n)		(fds->out + n)
 #define FDS_EX(fds, n)		(fds->ex + n)
@@ -644,10 +669,24 @@  int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
 	fds.res_out = bits + 4*size;
 	fds.res_ex  = bits + 5*size;
 
+#if defined(CONFIG_X86_X32)
+	if (test_thread_flag(TIF_X32)) {
+		if ((ret = compat_get_fd_set(n, (compat_ulong_t __user *)inp, fds.in)) ||
+		    (ret = compat_get_fd_set(n, (compat_ulong_t __user *)outp, fds.out)) ||
+		    (ret = compat_get_fd_set(n, (compat_ulong_t __user *)exp, fds.ex)))
+			goto out;
+	} else {
+		if ((ret = get_fd_set(n, inp, fds.in)) ||
+		    (ret = get_fd_set(n, outp, fds.out)) ||
+		    (ret = get_fd_set(n, exp, fds.ex)))
+			goto out;
+	}
+#else
 	if ((ret = get_fd_set(n, inp, fds.in)) ||
 	    (ret = get_fd_set(n, outp, fds.out)) ||
 	    (ret = get_fd_set(n, exp, fds.ex)))
 		goto out;
+#endif
 	zero_fd_set(n, fds.res_in);
 	zero_fd_set(n, fds.res_out);
 	zero_fd_set(n, fds.res_ex);
@@ -663,10 +702,24 @@  int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
 		ret = 0;
 	}
 
+#if defined(CONFIG_X86_X32)
+	if (test_thread_flag(TIF_X32)) {
+		if ((ret = compat_set_fd_set(n, (compat_ulong_t __user *)inp, fds.res_in)) ||
+		    (ret = compat_set_fd_set(n, (compat_ulong_t __user *)outp, fds.res_out)) ||
+		    (ret = compat_set_fd_set(n, (compat_ulong_t __user *)exp, fds.res_ex)))
+			ret = -EFAULT;
+	} else {
+		if (set_fd_set(n, inp, fds.res_in) ||
+		    set_fd_set(n, outp, fds.res_out) ||
+		    set_fd_set(n, exp, fds.res_ex))
+			ret = -EFAULT;
+	}
+#else
 	if (set_fd_set(n, inp, fds.res_in) ||
 	    set_fd_set(n, outp, fds.res_out) ||
 	    set_fd_set(n, exp, fds.res_ex))
 		ret = -EFAULT;
+#endif
 
 out:
 	if (bits != stack_fds)
@@ -1149,31 +1202,6 @@  int compat_poll_select_copy_remaining(struct timespec64 *end_time, void __user *
 	return ret;
 }
 
-/*
- * Ooo, nasty.  We need here to frob 32-bit unsigned longs to
- * 64-bit unsigned longs.
- */
-static
-int compat_get_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
-			unsigned long *fdset)
-{
-	if (ufdset) {
-		return compat_get_bitmap(fdset, ufdset, nr);
-	} else {
-		zero_fd_set(nr, fdset);
-		return 0;
-	}
-}
-
-static
-int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
-		      unsigned long *fdset)
-{
-	if (!ufdset)
-		return 0;
-	return compat_put_bitmap(ufdset, fdset, nr);
-}
-
 
 /*
  * This is a virtual copy of sys_select from fs/select.c and probably