From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753579AbdLMQ3s (ORCPT ); Wed, 13 Dec 2017 11:29:48 -0500 Received: from kanga.kvack.org ([205.233.56.17]:57050 "EHLO kanga.kvack.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750916AbdLMQ3r (ORCPT ); Wed, 13 Dec 2017 11:29:47 -0500 X-Greylist: delayed 1846 seconds by postgrey-1.27 at vger.kernel.org; Wed, 13 Dec 2017 11:29:47 EST Date: Wed, 13 Dec 2017 10:58:59 -0500 From: Benjamin LaHaise To: Matthew Wilcox Cc: Zhen Lei , Alexander Viro , linux-fsdevel , linux-aio , linux-kernel , Tianhong Ding , Hanjun Guo , Libin , Kefeng Wang , Deepa Dinamani Subject: Re: [PATCH 1/1] aio: make sure the input "timeout" value is valid Message-ID: <20171213155859.GB4403@kvack.org> References: <1513172572-16724-1-git-send-email-thunder.leizhen@huawei.com> <20171213141112.GA11217@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171213141112.GA11217@bombadil.infradead.org> User-Agent: Mutt/1.4.2.2i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Dec 13, 2017 at 06:11:12AM -0800, Matthew Wilcox wrote: > 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: That looks far better! Acked-by: Benjamin LaHaise -ben > 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); > -- "Thought is the essence of where you are now."