[PATCH 0/2] i/o: Make i/o y2038 safe

linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/2] i/o: Make i/o y2038 safe
@ 2017-08-05  4:12 Deepa Dinamani
  2017-08-05  4:12 ` [PATCH 1/2] select: Use get/put_timespec64 Deepa Dinamani
  2017-08-05  4:12 ` [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts Deepa Dinamani
  0 siblings, 2 replies; 6+ messages in thread
From: Deepa Dinamani @ 2017-08-05  4:12 UTC (permalink / raw)
  To: viro; +Cc: tglx, akpm, linux-fsdevel, linux-kernel, arnd, y2038, linux-aio

This is a preparatory series to make i/o y2038-safe by replacing
the use of struct timespec which is not y2038 safe by y2038 safe
struct timespec64.

Sockets and userspace interfaces themselves will be changed in
a separate series.

Deepa Dinamani (2):
  select: Use get/put_timespec64
  io_getevents: Use timespec64 to represent timeouts

 fs/aio.c    | 55 ++++++++++++++++++++++++++++++-------------------------
 fs/select.c | 60 ++++++++++++++++++++++++------------------------------------
 2 files changed, 54 insertions(+), 61 deletions(-)

-- 
2.11.0

Cc: linux-aio@kvack.org

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] select: Use get/put_timespec64
  2017-08-05  4:12 [PATCH 0/2] i/o: Make i/o y2038 safe Deepa Dinamani
@ 2017-08-05  4:12 ` Deepa Dinamani
  2017-08-05 19:53   ` Arnd Bergmann
  2017-08-05  4:12 ` [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts Deepa Dinamani
  1 sibling, 1 reply; 6+ messages in thread
From: Deepa Dinamani @ 2017-08-05  4:12 UTC (permalink / raw)
  To: viro; +Cc: tglx, akpm, linux-fsdevel, linux-kernel, arnd, y2038

Usage of these apis and their compat versions makes
the syscalls: select family of syscalls and their
compat implementations simpler.

This is a preparatory patch to isolate data conversions to
struct timespec64 at userspace boundaries. This helps contain
the changes needed to transition to new y2038 safe types.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/select.c | 60 ++++++++++++++++++++++++------------------------------------
 1 file changed, 24 insertions(+), 36 deletions(-)

diff --git a/fs/select.c b/fs/select.c
index 9d5f15ed87fe..0cf17fb33cf7 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -291,8 +291,7 @@ static int poll_select_copy_remaining(struct timespec64 *end_time,
 				      void __user *p,
 				      int timeval, int ret)
 {
-	struct timespec64 rts64;
-	struct timespec rts;
+	struct timespec64 rts;
 	struct timeval rtv;
 
 	if (!p)
@@ -305,23 +304,22 @@ static int poll_select_copy_remaining(struct timespec64 *end_time,
 	if (!end_time->tv_sec && !end_time->tv_nsec)
 		return ret;
 
-	ktime_get_ts64(&rts64);
-	rts64 = timespec64_sub(*end_time, rts64);
-	if (rts64.tv_sec < 0)
-		rts64.tv_sec = rts64.tv_nsec = 0;
+	ktime_get_ts64(&rts);
+	rts = timespec64_sub(*end_time, rts);
+	if (rts.tv_sec < 0)
+		rts.tv_sec = rts.tv_nsec = 0;
 
-	rts = timespec64_to_timespec(rts64);
 
 	if (timeval) {
 		if (sizeof(rtv) > sizeof(rtv.tv_sec) + sizeof(rtv.tv_usec))
 			memset(&rtv, 0, sizeof(rtv));
-		rtv.tv_sec = rts64.tv_sec;
-		rtv.tv_usec = rts64.tv_nsec / NSEC_PER_USEC;
+		rtv.tv_sec = rts.tv_sec;
+		rtv.tv_usec = rts.tv_nsec / NSEC_PER_USEC;
 
 		if (!copy_to_user(p, &rtv, sizeof(rtv)))
 			return ret;
 
-	} else if (!copy_to_user(p, &rts, sizeof(rts)))
+	} else if (!put_timespec64(&rts, p))
 		return ret;
 
 	/*
@@ -704,17 +702,15 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp,
 		       const sigset_t __user *sigmask, size_t sigsetsize)
 {
 	sigset_t ksigmask, sigsaved;
-	struct timespec ts;
-	struct timespec64 ts64, end_time, *to = NULL;
+	struct timespec64 ts, end_time, *to = NULL;
 	int ret;
 
 	if (tsp) {
-		if (copy_from_user(&ts, tsp, sizeof(ts)))
+		if (get_timespec64(&ts, tsp))
 			return -EFAULT;
-		ts64 = timespec_to_timespec64(ts);
 
 		to = &end_time;
-		if (poll_select_set_timeout(to, ts64.tv_sec, ts64.tv_nsec))
+		if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec))
 			return -EINVAL;
 	}
 
@@ -1051,12 +1047,11 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds,
 		size_t, sigsetsize)
 {
 	sigset_t ksigmask, sigsaved;
-	struct timespec ts;
-	struct timespec64 end_time, *to = NULL;
+	struct timespec64 ts, end_time, *to = NULL;
 	int ret;
 
 	if (tsp) {
-		if (copy_from_user(&ts, tsp, sizeof(ts)))
+		if (get_timespec64(&ts, tsp))
 			return -EFAULT;
 
 		to = &end_time;
@@ -1102,10 +1097,10 @@ SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds, unsigned int, nfds,
 #define __COMPAT_NFDBITS       (8 * sizeof(compat_ulong_t))
 
 static
-int compat_poll_select_copy_remaining(struct timespec *end_time, void __user *p,
+int compat_poll_select_copy_remaining(struct timespec64 *end_time, void __user *p,
 				      int timeval, int ret)
 {
-	struct timespec ts;
+	struct timespec64 ts;
 
 	if (!p)
 		return ret;
@@ -1117,8 +1112,8 @@ int compat_poll_select_copy_remaining(struct timespec *end_time, void __user *p,
 	if (!end_time->tv_sec && !end_time->tv_nsec)
 		return ret;
 
-	ktime_get_ts(&ts);
-	ts = timespec_sub(*end_time, ts);
+	ktime_get_ts64(&ts);
+	ts = timespec64_sub(*end_time, ts);
 	if (ts.tv_sec < 0)
 		ts.tv_sec = ts.tv_nsec = 0;
 
@@ -1131,12 +1126,7 @@ int compat_poll_select_copy_remaining(struct timespec *end_time, void __user *p,
 		if (!copy_to_user(p, &rtv, sizeof(rtv)))
 			return ret;
 	} else {
-		struct compat_timespec rts;
-
-		rts.tv_sec = ts.tv_sec;
-		rts.tv_nsec = ts.tv_nsec;
-
-		if (!copy_to_user(p, &rts, sizeof(rts)))
+		if (!compat_put_timespec64(&ts, p))
 			return ret;
 	}
 	/*
@@ -1198,7 +1188,7 @@ int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
  */
 static int compat_core_sys_select(int n, compat_ulong_t __user *inp,
 	compat_ulong_t __user *outp, compat_ulong_t __user *exp,
-	struct timespec *end_time)
+	struct timespec64 *end_time)
 {
 	fd_set_bits fds;
 	void *bits;
@@ -1271,7 +1261,7 @@ COMPAT_SYSCALL_DEFINE5(select, int, n, compat_ulong_t __user *, inp,
 	compat_ulong_t __user *, outp, compat_ulong_t __user *, exp,
 	struct compat_timeval __user *, tvp)
 {
-	struct timespec end_time, *to = NULL;
+	struct timespec64 end_time, *to = NULL;
 	struct compat_timeval tv;
 	int ret;
 
@@ -1317,12 +1307,11 @@ static long do_compat_pselect(int n, compat_ulong_t __user *inp,
 {
 	compat_sigset_t ss32;
 	sigset_t ksigmask, sigsaved;
-	struct compat_timespec ts;
-	struct timespec end_time, *to = NULL;
+	struct timespec64 ts, end_time, *to = NULL;
 	int ret;
 
 	if (tsp) {
-		if (copy_from_user(&ts, tsp, sizeof(ts)))
+		if (compat_get_timespec64(&ts, tsp))
 			return -EFAULT;
 
 		to = &end_time;
@@ -1386,12 +1375,11 @@ COMPAT_SYSCALL_DEFINE5(ppoll, struct pollfd __user *, ufds,
 {
 	compat_sigset_t ss32;
 	sigset_t ksigmask, sigsaved;
-	struct compat_timespec ts;
-	struct timespec end_time, *to = NULL;
+	struct timespec64 ts, end_time, *to = NULL;
 	int ret;
 
 	if (tsp) {
-		if (copy_from_user(&ts, tsp, sizeof(ts)))
+		if (compat_get_timespec64(&ts, tsp))
 			return -EFAULT;
 
 		to = &end_time;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] select: Use get/put_timespec64
  2017-08-05  4:12 ` [PATCH 1/2] select: Use get/put_timespec64 Deepa Dinamani
@ 2017-08-05 19:53   ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2017-08-05 19:53 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Al Viro, Thomas Gleixner, Andrew Morton,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	y2038 Mailman List

On Sat, Aug 5, 2017 at 6:12 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> Usage of these apis and their compat versions makes
> the syscalls: select family of syscalls and their
> compat implementations simpler.
>
> This is a preparatory patch to isolate data conversions to
> struct timespec64 at userspace boundaries. This helps contain
> the changes needed to transition to new y2038 safe types.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  fs/select.c | 60 ++++++++++++++++++++++++------------------------------------
>  1 file changed, 24 insertions(+), 36 deletions(-)

Looks all good to me,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts
  2017-08-05  4:12 [PATCH 0/2] i/o: Make i/o y2038 safe Deepa Dinamani
  2017-08-05  4:12 ` [PATCH 1/2] select: Use get/put_timespec64 Deepa Dinamani
@ 2017-08-05  4:12 ` Deepa Dinamani
  2017-08-05 20:13   ` Arnd Bergmann
  1 sibling, 1 reply; 6+ messages in thread
From: Deepa Dinamani @ 2017-08-05  4:12 UTC (permalink / raw)
  To: viro; +Cc: tglx, akpm, linux-fsdevel, linux-kernel, arnd, y2038, linux-aio

struct timespec is not y2038 safe. Use y2038 safe
struct timespec64 to represent timeouts.
The system call interface itself will be changed as
part of different series.

Timeouts will not really need more than 32 bits.
But, replacing these with timespec64 helps verification
of a y2038 safe kernel by getting rid of timespec
internally.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Cc: linux-aio@kvack.org
---
 fs/aio.c | 55 ++++++++++++++++++++++++++++++-------------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 8f0127526299..7ca6b7a00368 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1289,20 +1289,10 @@ static bool aio_read_events(struct kioctx *ctx, long min_nr, long nr,
 
 static long read_events(struct kioctx *ctx, long min_nr, long nr,
 			struct io_event __user *event,
-			struct timespec __user *timeout)
+			ktime_t until)
 {
-	ktime_t until = KTIME_MAX;
 	long ret = 0;
 
-	if (timeout) {
-		struct timespec	ts;
-
-		if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
-			return -EFAULT;
-
-		until = timespec_to_ktime(ts);
-	}
-
 	/*
 	 * Note that aio_read_events() is being called as the conditional - i.e.
 	 * we're calling it after prepare_to_wait() has set task state to
@@ -1824,6 +1814,25 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
 	return ret;
 }
 
+static long do_io_getevents(aio_context_t ctx_id,
+		long min_nr,
+		long nr,
+		struct io_event __user *events,
+		struct timespec64 *ts)
+{
+	ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
+	struct kioctx *ioctx = lookup_ioctx(ctx_id);
+	long ret = -EINVAL;
+
+	if (likely(ioctx)) {
+		if (likely(min_nr <= nr && min_nr >= 0))
+			ret = read_events(ioctx, min_nr, nr, events, until);
+		percpu_ref_put(&ioctx->users);
+	}
+
+	return ret;
+}
+
 /* io_getevents:
  *	Attempts to read at least min_nr events and up to nr events from
  *	the completion queue for the aio_context specified by ctx_id. If
@@ -1842,15 +1851,14 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
 		struct io_event __user *, events,
 		struct timespec __user *, timeout)
 {
-	struct kioctx *ioctx = lookup_ioctx(ctx_id);
-	long ret = -EINVAL;
+	struct timespec64	ts;
 
-	if (likely(ioctx)) {
-		if (likely(min_nr <= nr && min_nr >= 0))
-			ret = read_events(ioctx, min_nr, nr, events, timeout);
-		percpu_ref_put(&ioctx->users);
+	if (timeout) {
+		if (unlikely(get_timespec64(&ts, timeout)))
+			return -EFAULT;
 	}
-	return ret;
+
+	return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
 }
 
 #ifdef CONFIG_COMPAT
@@ -1860,17 +1868,14 @@ COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
 		       struct io_event __user *, events,
 		       struct compat_timespec __user *, timeout)
 {
-	struct timespec t;
-	struct timespec __user *ut = NULL;
+	struct timespec64 t;
 
 	if (timeout) {
-		if (compat_get_timespec(&t, timeout))
+		if (compat_get_timespec64(&t, timeout))
 			return -EFAULT;
 
-		ut = compat_alloc_user_space(sizeof(*ut));
-		if (copy_to_user(ut, &t, sizeof(t)))
-			return -EFAULT;
 	}
-	return sys_io_getevents(ctx_id, min_nr, nr, events, ut);
+
+	return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &t : NULL);
 }
 #endif
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts
  2017-08-05  4:12 ` [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts Deepa Dinamani
@ 2017-08-05 20:13   ` Arnd Bergmann
  2017-08-06  3:44     ` Deepa Dinamani
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2017-08-05 20:13 UTC (permalink / raw)
  To: Deepa Dinamani
  Cc: Al Viro, Thomas Gleixner, Andrew Morton,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	y2038 Mailman List, linux-aio, Benjamin LaHaise

On Sat, Aug 5, 2017 at 6:12 AM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> struct timespec is not y2038 safe. Use y2038 safe
> struct timespec64 to represent timeouts.
> The system call interface itself will be changed as
> part of different series.
>
> Timeouts will not really need more than 32 bits.
> But, replacing these with timespec64 helps verification
> of a y2038 safe kernel by getting rid of timespec
> internally.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Cc: linux-aio@kvack.org

Nice cleanup of the compat path!

>  static long read_events(struct kioctx *ctx, long min_nr, long nr,
>                         struct io_event __user *event,
> -                       struct timespec __user *timeout)
> +                       ktime_t until)
>  {
> -       ktime_t until = KTIME_MAX;
>         long ret = 0;
>
> -       if (timeout) {
> -               struct timespec ts;
> -
> -               if (unlikely(copy_from_user(&ts, timeout, sizeof(ts))))
> -                       return -EFAULT;
> -
> -               until = timespec_to_ktime(ts);
> -       }
> -
>         /*
>          * Note that aio_read_events() is being called as the conditional - i.e.
>          * we're calling it after prepare_to_wait() has set task state to
> @@ -1824,6 +1814,25 @@ SYSCALL_DEFINE3(io_cancel, aio_context_t, ctx_id, struct iocb __user *, iocb,
>         return ret;
>  }
>
> +static long do_io_getevents(aio_context_t ctx_id,
> +               long min_nr,
> +               long nr,
> +               struct io_event __user *events,
> +               struct timespec64 *ts)
> +{
> +       ktime_t until = ts ? timespec64_to_ktime(*ts) : KTIME_MAX;
> +       struct kioctx *ioctx = lookup_ioctx(ctx_id);
> +       long ret = -EINVAL;
> +
> +       if (likely(ioctx)) {
> +               if (likely(min_nr <= nr && min_nr >= 0))
> +                       ret = read_events(ioctx, min_nr, nr, events, until);
> +               percpu_ref_put(&ioctx->users);
> +       }
> +
> +       return ret;
> +}

I guess these two functions are small enough that they could be merged
into one, saving a few lines. Then again, fs/aio.c seems to generally use
fairly short functions doing not too much at once, so your approach maybe
fits better with the style of the subsystem.

Either way,

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

       Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts
  2017-08-05 20:13   ` Arnd Bergmann
@ 2017-08-06  3:44     ` Deepa Dinamani
  0 siblings, 0 replies; 6+ messages in thread
From: Deepa Dinamani @ 2017-08-06  3:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Al Viro, Thomas Gleixner, Andrew Morton,
	Linux FS-devel Mailing List, Linux Kernel Mailing List,
	y2038 Mailman List, linux-aio, Benjamin LaHaise

> I guess these two functions are small enough that they could be merged
> into one, saving a few lines. Then again, fs/aio.c seems to generally use
> fairly short functions doing not too much at once, so your approach maybe
> fits better with the style of the subsystem.

I don't see a problem with combining the two functions either.
Unless someone has a strong preference, I will leave it the way it is
currently handled.

Thanks,
Deepa

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-08-06  3:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-05  4:12 [PATCH 0/2] i/o: Make i/o y2038 safe Deepa Dinamani
2017-08-05  4:12 ` [PATCH 1/2] select: Use get/put_timespec64 Deepa Dinamani
2017-08-05 19:53   ` Arnd Bergmann
2017-08-05  4:12 ` [PATCH 2/2] io_getevents: Use timespec64 to represent timeouts Deepa Dinamani
2017-08-05 20:13   ` Arnd Bergmann
2017-08-06  3:44     ` Deepa Dinamani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).