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

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

Date:   Wed, 13 Dec 2017 06:11:12 -0800
From:   Matthew Wilcox <willy@...radead.org>
To:     Zhen Lei <thunder.leizhen@...wei.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Benjamin LaHaise <bcrl@...ck.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-aio <linux-aio@...ck.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Tianhong Ding <dingtianhong@...wei.com>,
        Hanjun Guo <guohanjun@...wei.com>,
        Libin <huawei.libin@...wei.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Deepa Dinamani <deepa.kernel@...il.com>
Subject: Re: [PATCH 1/1] aio: make sure the input "timeout" value is valid

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);