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 <lance.richardson.net@xxxxxxxxx>
---
v2: Fix build breakage when CONFIG_COMPAT is not selected (found by
kbuild test robot).
fs/select.c | 80 ++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 55 insertions(+), 25 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index b6c36254028a..cf4cb5aef6d7 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -384,6 +384,33 @@ void zero_fd_set(unsigned long nr, unsigned long *fdset)
memset(fdset, 0, FDS_BYTES(nr));
}
+#if defined(CONFIG_COMPAT)
+/*
+ * 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);
+}
+#endif
+
#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 +671,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 +704,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 +1204,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
--
2.14.1
