Re: [PATCH 1/1] aio: make sure the input "timeout" value is valid

From: Matthew Wilcox
Date: Wed Dec 13 2017 - 09:11:26 EST


On Wed, Dec 13, 2017 at 09:42:52PM +0800, Zhen Lei wrote:
> Below information is reported by a lower kernel version, and I saw the
> problem still exist in current version.

I think you're right, but what an awful interface we have here!
The user must not only fetch it, they must validate it separately?
And if they forget, then userspace is provoking undefined behaviour? Ugh.
Why not this:

diff --git a/fs/aio.c b/fs/aio.c
index 4adbdcbe753a..fdd16cf897c8 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1788,8 +1788,9 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
struct timespec64 ts;

if (timeout) {
- if (unlikely(get_timespec64(&ts, timeout)))
- return -EFAULT;
+ int error = get_valid_timespec64(&ts, timeout);
+ if (error)
+ return error;
}

return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
@@ -1805,9 +1806,9 @@ COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
struct timespec64 t;

if (timeout) {
- if (compat_get_timespec64(&t, timeout))
- return -EFAULT;
-
+ int error = compat_get_valid_timespec64(&t, timeout);
+ if (error)
+ return error;
}

return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 0fc36406f32c..578fc0f208d9 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -172,6 +172,26 @@ extern int get_compat_itimerspec64(struct itimerspec64 *its,
extern int put_compat_itimerspec64(const struct itimerspec64 *its,
struct compat_itimerspec __user *uits);

+static inline __must_check
+int compat_get_valid_timespec64(struct timespec64 *ts, const void __user *uts)
+{
+ if (unlikely(compat_get_timespec64(ts, uts)))
+ return -EFAULT;
+ if (unlikely(!timespec64_valid(ts)))
+ return -EINVAL;
+ return 0;
+}
+
+static inline __must_check
+int compat_get_strict_timespec64(struct timespec64 *ts, const void __user *uts)
+{
+ if (unlikely(compat_get_timespec64(ts, uts)))
+ return -EFAULT;
+ if (unlikely(!timespec64_valid_strict(ts)))
+ return -EINVAL;
+ return 0;
+}
+
struct compat_iovec {
compat_uptr_t iov_base;
compat_size_t iov_len;
diff --git a/include/linux/time.h b/include/linux/time.h
index 4b62a2c0a661..506d87483d04 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -18,6 +18,26 @@ int get_itimerspec64(struct itimerspec64 *it,
int put_itimerspec64(const struct itimerspec64 *it,
struct itimerspec __user *uit);

+static inline __must_check int get_valid_timespec64(struct timespec64 *ts,
+ const struct timespec __user *uts)
+{
+ if (unlikely(get_timespec64(ts, uts)))
+ return -EFAULT;
+ if (unlikely(!timespec64_valid(ts)))
+ return -EINVAL;
+ return 0;
+}
+
+static inline __must_check int get_strict_timespec64(struct timespec64 *ts,
+ const struct timespec __user *uts)
+{
+ if (unlikely(get_timespec64(ts, uts)))
+ return -EFAULT;
+ if (unlikely(!timespec64_valid_strict(ts)))
+ return -EINVAL;
+ return 0;
+}
+
extern time64_t mktime64(const unsigned int year, const unsigned int mon,
const unsigned int day, const unsigned int hour,
const unsigned int min, const unsigned int sec);