Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1674128 > unrolled thread

[PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-24 21:00 +0200
    [PATCH v3 5/7] nanosleep: Use get_timepsec64() and put_timespec64() Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-24 21:00 +0200
    [PATCH v3 7/7] posix_clocks: Use get_itimerspec64() and  put_itimerspec64() Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-24 21:00 +0200
    [PATCH v3 3/7] posix-stubs: Conditionally include COMPAT_SYS_NI defines Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-24 21:00 +0200
    [PATCH v3 6/7] timerfd: Use get_itimerspec64() and put_itimerspec64() Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-24 21:00 +0200
    Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Al Viro <viro@ZenIV.linux.org.uk> - 2017-06-26 04:40 +0200
      Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Al Viro <viro@ZenIV.linux.org.uk> - 2017-06-26 06:20 +0200
      Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-26 20:20 +0200
        Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Arnd Bergmann <arnd@arndb.de> - 2017-06-26 22:10 +0200
          Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Deepa Dinamani <deepa.kernel@gmail.com> - 2017-06-27 07:00 +0200
          Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer  syscalls Thomas Gleixner <tglx@linutronix.de> - 2017-07-03 12:30 +0200
            Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls Arnd Bergmann <arnd@arndb.de> - 2017-07-03 13:30 +0200

#1674128 — [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls

The series aims at isolating data conversions of time_t based structures:
struct timespec and struct itimerspec at user space boundaries.
This helps to later change the underlying types to handle y2038 changes
to these.

The series is an update to Arnd Bergmann's previous series:
http://sourceware.org/ml/libc-alpha/2015-05/msg00070.html

Changes since v1:
* Rebased and removed common code paths on the tip linux-next.
Changes since v2:
* Removed accidental inclusion of wrong version of patch 3/7.

Deepa Dinamani (7):
  time: add get_timespec64 and put_timespec64
  time: introduce {get,put}_itimerspec64
  posix-stubs: Conditionally include COMPAT_SYS_NI defines
  posix-timers: Use get_timepsec64() and put_timespec64()
  nanosleep: Use get_timepsec64() and put_timespec64()
  timerfd: Use get_itimerspec64() and put_itimerspec64()
  posix_clocks: Use get_itimerspec64() and  put_itimerspec64()

 fs/timerfd.c                   |  43 +++++++------
 include/linux/compat.h         |   6 ++
 include/linux/hrtimer.h        |   2 +-
 include/linux/posix-timers.h   |   1 -
 include/linux/time.h           |  18 ++++++
 kernel/compat.c                |  65 ++++++++++++++++++++
 kernel/time/alarmtimer.c       |   4 +-
 kernel/time/hrtimer.c          |  30 ++++------
 kernel/time/posix-cpu-timers.c |   8 +--
 kernel/time/posix-stubs.c      |  96 +++++++++++++++--------------
 kernel/time/posix-timers.c     | 133 +++++++++++++++--------------------------
 kernel/time/time.c             |  58 ++++++++++++++++++
 12 files changed, 287 insertions(+), 177 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1674129 — [PATCH v3 5/7] nanosleep: Use get_timepsec64() and put_timespec64()

Usage of these apis and their compat versions makes
the syscalls: clock_nanosleep and nanosleep 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>
---
 include/linux/hrtimer.h        |  2 +-
 kernel/time/alarmtimer.c       |  4 ++--
 kernel/time/hrtimer.c          | 30 +++++++++++++-----------------
 kernel/time/posix-cpu-timers.c |  8 ++------
 kernel/time/posix-timers.c     | 20 ++++++++------------
 5 files changed, 26 insertions(+), 38 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 255edd5e7a74..012c37fdb688 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -453,7 +453,7 @@ static inline u64 hrtimer_forward_now(struct hrtimer *timer,
 
 /* Precise sleep: */
 
-extern int nanosleep_copyout(struct restart_block *, struct timespec *);
+extern int nanosleep_copyout(struct restart_block *, struct timespec64 *);
 extern long hrtimer_nanosleep(const struct timespec64 *rqtp,
 			      const enum hrtimer_mode mode,
 			      const clockid_t clockid);
diff --git a/kernel/time/alarmtimer.c b/kernel/time/alarmtimer.c
index c991cf212c6d..0b8ff7d257ea 100644
--- a/kernel/time/alarmtimer.c
+++ b/kernel/time/alarmtimer.c
@@ -712,14 +712,14 @@ static int alarmtimer_do_nsleep(struct alarm *alarm, ktime_t absexp,
 		alarmtimer_freezerset(absexp, type);
 	restart = &current->restart_block;
 	if (restart->nanosleep.type != TT_NONE) {
-		struct timespec rmt;
+		struct timespec64 rmt;
 		ktime_t rem;
 
 		rem = ktime_sub(absexp, alarm_bases[type].gettime());
 
 		if (rem <= 0)
 			return 0;
-		rmt = ktime_to_timespec(rem);
+		rmt = ktime_to_timespec64(rem);
 
 		return nanosleep_copyout(restart, &rmt);
 	}
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 81da124f1115..88f75f92ef36 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1440,17 +1440,17 @@ void hrtimer_init_sleeper(struct hrtimer_sleeper *sl, struct task_struct *task)
 }
 EXPORT_SYMBOL_GPL(hrtimer_init_sleeper);
 
-int nanosleep_copyout(struct restart_block *restart, struct timespec *ts)
+int nanosleep_copyout(struct restart_block *restart, struct timespec64 *ts)
 {
 	switch(restart->nanosleep.type) {
 #ifdef CONFIG_COMPAT
 	case TT_COMPAT:
-		if (compat_put_timespec(ts, restart->nanosleep.compat_rmtp))
+		if (compat_put_timespec64(ts, restart->nanosleep.compat_rmtp))
 			return -EFAULT;
 		break;
 #endif
 	case TT_NATIVE:
-		if (copy_to_user(restart->nanosleep.rmtp, ts, sizeof(struct timespec)))
+		if (put_timespec64(ts, restart->nanosleep.rmtp))
 			return -EFAULT;
 		break;
 	default:
@@ -1485,11 +1485,11 @@ static int __sched do_nanosleep(struct hrtimer_sleeper *t, enum hrtimer_mode mod
 	restart = &current->restart_block;
 	if (restart->nanosleep.type != TT_NONE) {
 		ktime_t rem = hrtimer_expires_remaining(&t->timer);
-		struct timespec rmt;
+		struct timespec64 rmt;
 
 		if (rem <= 0)
 			return 0;
-		rmt = ktime_to_timespec(rem);
+		rmt = ktime_to_timespec64(rem);
 
 		return nanosleep_copyout(restart, &rmt);
 	}
@@ -1546,19 +1546,17 @@ long hrtimer_nanosleep(const struct timespec64 *rqtp,
 SYSCALL_DEFINE2(nanosleep, struct timespec __user *, rqtp,
 		struct timespec __user *, rmtp)
 {
-	struct timespec64 tu64;
-	struct timespec tu;
+	struct timespec64 tu;
 
-	if (copy_from_user(&tu, rqtp, sizeof(tu)))
+	if (get_timespec64(&tu, rqtp))
 		return -EFAULT;
 
-	tu64 = timespec_to_timespec64(tu);
-	if (!timespec64_valid(&tu64))
+	if (!timespec64_valid(&tu))
 		return -EINVAL;
 
 	current->restart_block.nanosleep.type = rmtp ? TT_NATIVE : TT_NONE;
 	current->restart_block.nanosleep.rmtp = rmtp;
-	return hrtimer_nanosleep(&tu64, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
+	return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
 }
 
 #ifdef CONFIG_COMPAT
@@ -1566,19 +1564,17 @@ SYSCALL_DEFINE2(nanosleep, struct timespec __user *, rqtp,
 COMPAT_SYSCALL_DEFINE2(nanosleep, struct compat_timespec __user *, rqtp,
 		       struct compat_timespec __user *, rmtp)
 {
-	struct timespec64 tu64;
-	struct timespec tu;
+	struct timespec64 tu;
 
-	if (compat_get_timespec(&tu, rqtp))
+	if (compat_get_timespec64(&tu, rqtp))
 		return -EFAULT;
 
-	tu64 = timespec_to_timespec64(tu);
-	if (!timespec64_valid(&tu64))
+	if (!timespec64_valid(&tu))
 		return -EINVAL;
 
 	current->restart_block.nanosleep.type = rmtp ? TT_COMPAT : TT_NONE;
 	current->restart_block.nanosleep.compat_rmtp = rmtp;
-	return hrtimer_nanosleep(&tu64, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
+	return hrtimer_nanosleep(&tu, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
 }
 #endif
 
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 60cb24ac9ebc..a3bd5dbe0dc4 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -1318,12 +1318,8 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
 		 */
 		restart = &current->restart_block;
 		restart->nanosleep.expires = expires;
-		if (restart->nanosleep.type != TT_NONE) {
-			struct timespec ts;
-
-			ts = timespec64_to_timespec(it.it_value);
-			error = nanosleep_copyout(restart, &ts);
-		}
+		if (restart->nanosleep.type != TT_NONE)
+			error = nanosleep_copyout(restart, &it.it_value);
 	}
 
 	return error;
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 8717f4bc7182..6818818caffd 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -1207,26 +1207,24 @@ SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
 		struct timespec __user *, rmtp)
 {
 	const struct k_clock *kc = clockid_to_kclock(which_clock);
-	struct timespec64 t64;
-	struct timespec t;
+	struct timespec64 t;
 
 	if (!kc)
 		return -EINVAL;
 	if (!kc->nsleep)
 		return -ENANOSLEEP_NOTSUP;
 
-	if (copy_from_user(&t, rqtp, sizeof (struct timespec)))
+	if (get_timespec64(&t, rqtp))
 		return -EFAULT;
 
-	t64 = timespec_to_timespec64(t);
-	if (!timespec64_valid(&t64))
+	if (!timespec64_valid(&t))
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
 	current->restart_block.nanosleep.type = rmtp ? TT_NATIVE : TT_NONE;
 	current->restart_block.nanosleep.rmtp = rmtp;
 
-	return kc->nsleep(which_clock, flags, &t64);
+	return kc->nsleep(which_clock, flags, &t);
 }
 
 #ifdef CONFIG_COMPAT
@@ -1235,26 +1233,24 @@ COMPAT_SYSCALL_DEFINE4(clock_nanosleep, clockid_t, which_clock, int, flags,
 		       struct compat_timespec __user *, rmtp)
 {
 	const struct k_clock *kc = clockid_to_kclock(which_clock);
-	struct timespec64 t64;
-	struct timespec t;
+	struct timespec64 t;
 
 	if (!kc)
 		return -EINVAL;
 	if (!kc->nsleep)
 		return -ENANOSLEEP_NOTSUP;
 
-	if (compat_get_timespec(&t, rqtp))
+	if (compat_get_timespec64(&t, rqtp))
 		return -EFAULT;
 
-	t64 = timespec_to_timespec64(t);
-	if (!timespec64_valid(&t64))
+	if (!timespec64_valid(&t))
 		return -EINVAL;
 	if (flags & TIMER_ABSTIME)
 		rmtp = NULL;
 	current->restart_block.nanosleep.type = rmtp ? TT_COMPAT : TT_NONE;
 	current->restart_block.nanosleep.compat_rmtp = rmtp;
 
-	return kc->nsleep(which_clock, flags, &t64);
+	return kc->nsleep(which_clock, flags, &t);
 }
 #endif
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1674130 — [PATCH v3 7/7] posix_clocks: Use get_itimerspec64() and put_itimerspec64()

Usage of these apis and their compat versions makes
the syscalls: timer_settime and timer_gettime and their
compat implementations simpler.

This patch also serves as a preparatory patch for changing
syscalls to use new time_t data types to support the
y2038 effort by isolating the processing of user pointers
through these apis.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 kernel/time/posix-timers.c | 44 ++++++++++++++++----------------------------
 1 file changed, 16 insertions(+), 28 deletions(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 6818818caffd..5bcc87e19c97 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -739,13 +739,11 @@ static int do_timer_gettime(timer_t timer_id,  struct itimerspec64 *setting)
 SYSCALL_DEFINE2(timer_gettime, timer_t, timer_id,
 		struct itimerspec __user *, setting)
 {
-	struct itimerspec64 cur_setting64;
+	struct itimerspec64 cur_setting;
 
-	int ret = do_timer_gettime(timer_id, &cur_setting64);
+	int ret = do_timer_gettime(timer_id, &cur_setting);
 	if (!ret) {
-		struct itimerspec cur_setting;
-		cur_setting = itimerspec64_to_itimerspec(&cur_setting64);
-		if (copy_to_user(setting, &cur_setting, sizeof (cur_setting)))
+		if (put_itimerspec64(&cur_setting, setting))
 			ret = -EFAULT;
 	}
 	return ret;
@@ -755,13 +753,11 @@ SYSCALL_DEFINE2(timer_gettime, timer_t, timer_id,
 COMPAT_SYSCALL_DEFINE2(timer_gettime, timer_t, timer_id,
 		       struct compat_itimerspec __user *, setting)
 {
-	struct itimerspec64 cur_setting64;
+	struct itimerspec64 cur_setting;
 
-	int ret = do_timer_gettime(timer_id, &cur_setting64);
+	int ret = do_timer_gettime(timer_id, &cur_setting);
 	if (!ret) {
-		struct itimerspec cur_setting;
-		cur_setting = itimerspec64_to_itimerspec(&cur_setting64);
-		if (put_compat_itimerspec(setting, &cur_setting))
+		if (put_compat_itimerspec64(&cur_setting, setting))
 			ret = -EFAULT;
 	}
 	return ret;
@@ -907,23 +903,19 @@ SYSCALL_DEFINE4(timer_settime, timer_t, timer_id, int, flags,
 		const struct itimerspec __user *, new_setting,
 		struct itimerspec __user *, old_setting)
 {
-	struct itimerspec64 new_spec64, old_spec64;
-	struct itimerspec64 *rtn = old_setting ? &old_spec64 : NULL;
-	struct itimerspec new_spec;
+	struct itimerspec64 new_spec, old_spec;
+	struct itimerspec64 *rtn = old_setting ? &old_spec : NULL;
 	int error = 0;
 
 	if (!new_setting)
 		return -EINVAL;
 
-	if (copy_from_user(&new_spec, new_setting, sizeof (new_spec)))
+	if (get_itimerspec64(&new_spec, new_setting))
 		return -EFAULT;
-	new_spec64 = itimerspec_to_itimerspec64(&new_spec);
 
-	error = do_timer_settime(timer_id, flags, &new_spec64, rtn);
+	error = do_timer_settime(timer_id, flags, &new_spec, rtn);
 	if (!error && old_setting) {
-		struct itimerspec old_spec;
-		old_spec = itimerspec64_to_itimerspec(&old_spec64);
-		if (copy_to_user(old_setting, &old_spec, sizeof (old_spec)))
+		if (put_itimerspec64(&old_spec, old_setting))
 			error = -EFAULT;
 	}
 	return error;
@@ -934,22 +926,18 @@ COMPAT_SYSCALL_DEFINE4(timer_settime, timer_t, timer_id, int, flags,
 		       struct compat_itimerspec __user *, new,
 		       struct compat_itimerspec __user *, old)
 {
-	struct itimerspec64 new_spec64, old_spec64;
-	struct itimerspec64 *rtn = old ? &old_spec64 : NULL;
-	struct itimerspec new_spec;
+	struct itimerspec64 new_spec, old_spec;
+	struct itimerspec64 *rtn = old ? &old_spec : NULL;
 	int error = 0;
 
 	if (!new)
 		return -EINVAL;
-	if (get_compat_itimerspec(&new_spec, new))
+	if (get_compat_itimerspec64(&new_spec, new))
 		return -EFAULT;
 
-	new_spec64 = itimerspec_to_itimerspec64(&new_spec);
-	error = do_timer_settime(timer_id, flags, &new_spec64, rtn);
+	error = do_timer_settime(timer_id, flags, &new_spec, rtn);
 	if (!error && old) {
-		struct itimerspec old_spec;
-		old_spec = itimerspec64_to_itimerspec(&old_spec64);
-		if (put_compat_itimerspec(old, &old_spec))
+		if (put_compat_itimerspec64(&old_spec, old))
 			error = -EFAULT;
 	}
 	return error;
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1674131 — [PATCH v3 3/7] posix-stubs: Conditionally include COMPAT_SYS_NI defines

These apis only need to be defined if CONFIG_COMPAT is
enabled.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 kernel/time/posix-stubs.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/kernel/time/posix-stubs.c b/kernel/time/posix-stubs.c
index 38f3b20efa29..65878221cbfb 100644
--- a/kernel/time/posix-stubs.c
+++ b/kernel/time/posix-stubs.c
@@ -41,12 +41,6 @@ SYS_NI(setitimer);
 #ifdef __ARCH_WANT_SYS_ALARM
 SYS_NI(alarm);
 #endif
-COMPAT_SYS_NI(timer_create);
-COMPAT_SYS_NI(clock_adjtime);
-COMPAT_SYS_NI(timer_settime);
-COMPAT_SYS_NI(timer_gettime);
-COMPAT_SYS_NI(getitimer);
-COMPAT_SYS_NI(setitimer);
 
 /*
  * We preserve minimal support for CLOCK_REALTIME and CLOCK_MONOTONIC
@@ -138,6 +132,13 @@ SYSCALL_DEFINE4(clock_nanosleep, const clockid_t, which_clock, int, flags,
 }
 
 #ifdef CONFIG_COMPAT
+COMPAT_SYS_NI(timer_create);
+COMPAT_SYS_NI(clock_adjtime);
+COMPAT_SYS_NI(timer_settime);
+COMPAT_SYS_NI(timer_gettime);
+COMPAT_SYS_NI(getitimer);
+COMPAT_SYS_NI(setitimer);
+
 COMPAT_SYSCALL_DEFINE2(clock_settime, const clockid_t, which_clock,
 		       struct compat_timespec __user *, tp)
 {
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1674132 — [PATCH v3 6/7] timerfd: Use get_itimerspec64() and put_itimerspec64()

Usage of these apis and their compat versions makes
the syscalls: timerfd_settime and timerfd_gettime and
their compat implementations simpler.

This patch also serves as a preparatory patch for changing
syscalls to use new time_t data types to support the
y2038 effort by isolating the processing of user pointers
through these apis.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 fs/timerfd.c | 43 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index c543cdb5f8ed..ece0c02d7e63 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -169,7 +169,7 @@ static ktime_t timerfd_get_remaining(struct timerfd_ctx *ctx)
 }
 
 static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
-			 const struct itimerspec *ktmr)
+			 const struct itimerspec64 *ktmr)
 {
 	enum hrtimer_mode htmode;
 	ktime_t texp;
@@ -178,10 +178,10 @@ static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
 	htmode = (flags & TFD_TIMER_ABSTIME) ?
 		HRTIMER_MODE_ABS: HRTIMER_MODE_REL;
 
-	texp = timespec_to_ktime(ktmr->it_value);
+	texp = timespec64_to_ktime(ktmr->it_value);
 	ctx->expired = 0;
 	ctx->ticks = 0;
-	ctx->tintv = timespec_to_ktime(ktmr->it_interval);
+	ctx->tintv = timespec64_to_ktime(ktmr->it_interval);
 
 	if (isalarm(ctx)) {
 		alarm_init(&ctx->t.alarm,
@@ -432,16 +432,15 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 }
 
 static int do_timerfd_settime(int ufd, int flags, 
-		const struct itimerspec *new,
-		struct itimerspec *old)
+		const struct itimerspec64 *new,
+		struct itimerspec64 *old)
 {
 	struct fd f;
 	struct timerfd_ctx *ctx;
 	int ret;
 
 	if ((flags & ~TFD_SETTIME_FLAGS) ||
-	    !timespec_valid(&new->it_value) ||
-	    !timespec_valid(&new->it_interval))
+		 !itimerspec64_valid(new))
 		return -EINVAL;
 
 	ret = timerfd_fget(ufd, &f);
@@ -487,8 +486,8 @@ static int do_timerfd_settime(int ufd, int flags,
 			hrtimer_forward_now(&ctx->t.tmr, ctx->tintv);
 	}
 
-	old->it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
-	old->it_interval = ktime_to_timespec(ctx->tintv);
+	old->it_value = ktime_to_timespec64(timerfd_get_remaining(ctx));
+	old->it_interval = ktime_to_timespec64(ctx->tintv);
 
 	/*
 	 * Re-program the timer to the new value ...
@@ -500,7 +499,7 @@ static int do_timerfd_settime(int ufd, int flags,
 	return ret;
 }
 
-static int do_timerfd_gettime(int ufd, struct itimerspec *t)
+static int do_timerfd_gettime(int ufd, struct itimerspec64 *t)
 {
 	struct fd f;
 	struct timerfd_ctx *ctx;
@@ -525,8 +524,8 @@ static int do_timerfd_gettime(int ufd, struct itimerspec *t)
 			hrtimer_restart(&ctx->t.tmr);
 		}
 	}
-	t->it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
-	t->it_interval = ktime_to_timespec(ctx->tintv);
+	t->it_value = ktime_to_timespec64(timerfd_get_remaining(ctx));
+	t->it_interval = ktime_to_timespec64(ctx->tintv);
 	spin_unlock_irq(&ctx->wqh.lock);
 	fdput(f);
 	return 0;
@@ -536,15 +535,15 @@ SYSCALL_DEFINE4(timerfd_settime, int, ufd, int, flags,
 		const struct itimerspec __user *, utmr,
 		struct itimerspec __user *, otmr)
 {
-	struct itimerspec new, old;
+	struct itimerspec64 new, old;
 	int ret;
 
-	if (copy_from_user(&new, utmr, sizeof(new)))
+	if (get_itimerspec64(&new, utmr))
 		return -EFAULT;
 	ret = do_timerfd_settime(ufd, flags, &new, &old);
 	if (ret)
 		return ret;
-	if (otmr && copy_to_user(otmr, &old, sizeof(old)))
+	if (otmr && put_itimerspec64(&old, otmr))
 		return -EFAULT;
 
 	return ret;
@@ -552,11 +551,11 @@ SYSCALL_DEFINE4(timerfd_settime, int, ufd, int, flags,
 
 SYSCALL_DEFINE2(timerfd_gettime, int, ufd, struct itimerspec __user *, otmr)
 {
-	struct itimerspec kotmr;
+	struct itimerspec64 kotmr;
 	int ret = do_timerfd_gettime(ufd, &kotmr);
 	if (ret)
 		return ret;
-	return copy_to_user(otmr, &kotmr, sizeof(kotmr)) ? -EFAULT: 0;
+	return put_itimerspec64(&kotmr, otmr) ? -EFAULT : 0;
 }
 
 #ifdef CONFIG_COMPAT
@@ -564,15 +563,15 @@ COMPAT_SYSCALL_DEFINE4(timerfd_settime, int, ufd, int, flags,
 		const struct compat_itimerspec __user *, utmr,
 		struct compat_itimerspec __user *, otmr)
 {
-	struct itimerspec new, old;
+	struct itimerspec64 new, old;
 	int ret;
 
-	if (get_compat_itimerspec(&new, utmr))
+	if (get_compat_itimerspec64(&new, utmr))
 		return -EFAULT;
 	ret = do_timerfd_settime(ufd, flags, &new, &old);
 	if (ret)
 		return ret;
-	if (otmr && put_compat_itimerspec(otmr, &old))
+	if (otmr && put_compat_itimerspec64(&old, otmr))
 		return -EFAULT;
 	return ret;
 }
@@ -580,10 +579,10 @@ COMPAT_SYSCALL_DEFINE4(timerfd_settime, int, ufd, int, flags,
 COMPAT_SYSCALL_DEFINE2(timerfd_gettime, int, ufd,
 		struct compat_itimerspec __user *, otmr)
 {
-	struct itimerspec kotmr;
+	struct itimerspec64 kotmr;
 	int ret = do_timerfd_gettime(ufd, &kotmr);
 	if (ret)
 		return ret;
-	return put_compat_itimerspec(otmr, &kotmr) ? -EFAULT: 0;
+	return put_compat_itimerspec64(&kotmr, otmr) ? -EFAULT : 0;
 }
 #endif
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1674405

On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
> The series aims at isolating data conversions of time_t based structures:
> struct timespec and struct itimerspec at user space boundaries.
> This helps to later change the underlying types to handle y2038 changes
> to these.

Nice...  A few questions:

* what about setitimer(2)?  Right now that's the only remaining user of
get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().

* you have two callers of get_compat_itimerspec64(); one is followed by
itimerspec64_valid(), another - by its open-coded analogue.  The same
goes for get_itimerspec64(); wouldn't it be better to have both check
the validity immediately and simply fail with -EINVAL?  Matter of taste,
but...

* should __sys_recvmmsg() switch to timespec64?

[toc] | [prev] | [next] | [standalone]

#1674425

On Mon, Jun 26, 2017 at 03:35:25AM +0100, Al Viro wrote:
> On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
> > The series aims at isolating data conversions of time_t based structures:
> > struct timespec and struct itimerspec at user space boundaries.
> > This helps to later change the underlying types to handle y2038 changes
> > to these.
> 
> Nice...  A few questions:
> 
> * what about setitimer(2)?  Right now that's the only remaining user of
> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().
> 
> * you have two callers of get_compat_itimerspec64(); one is followed by
> itimerspec64_valid(), another - by its open-coded analogue.  The same
> goes for get_itimerspec64(); wouldn't it be better to have both check
> the validity immediately and simply fail with -EINVAL?  Matter of taste,
> but...
> 
> * should __sys_recvmmsg() switch to timespec64?

While we are at it - do we need any locking for accesses of ->sk_stamp?
	* ax25, ipx, netrom, qrtr: sock_get_timestamp() done under lock_sock().
	* bluetooth: without (and case next door in the same switch is
grabbing/dropping lock_sock, so it's not held by caller either)
	* ipv4, ipv6, packet, can: without
	* irda: without, checks for NULL sock->sk for some reason (other
cases do not, so if we ever get there with NULL ->sk, we are fucked).
Incidentally, TIOCINQ in there looks fishy - what's to prevent us from
losing CPU just as skb_peek() returns, with skb getting freed by the
time we regain it and go looking at skb->len?  Don't we need at least
to hold ->lock on queue we are peeking into?
	* rose: without, and TIOCINQ there looks similar to irda one
	* x25: without, with the same odd check for NULL sock->sk
	* atm: without, apparently.  Same unprotected skb_peek() on
TIOCINQ...
	* atalk: ditto.

[toc] | [prev] | [next] | [standalone]

#1674989

On Sun, Jun 25, 2017 at 7:35 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
>> The series aims at isolating data conversions of time_t based structures:
>> struct timespec and struct itimerspec at user space boundaries.
>> This helps to later change the underlying types to handle y2038 changes
>> to these.
>
> Nice...  A few questions:
>
> * what about setitimer(2)?  Right now that's the only remaining user of
> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().

We do not plan to support these beyond y2038 on 32 bit systems.
timer_settime() and timer_gettime() are considered to be replacements
for these, respectively.

There is also going to be a cleanup of timeval/ timespec/ time_t data
types and apis after the new syscalls are ready.
At that time I might choose to get rid of these itimerval apis. I'm
not sure yet.

> * you have two callers of get_compat_itimerspec64(); one is followed by
> itimerspec64_valid(), another - by its open-coded analogue.  The same
> goes for get_itimerspec64(); wouldn't it be better to have both check
> the validity immediately and simply fail with -EINVAL?  Matter of taste,
> but...

This is what I thought also. And, in fact this is how I had it in one
of the earlier version of my series.
But, the utimensat(2) is what I consider provides a counter example of
why this is a bad idea.
There is no reason you should not be able to read the itimerspec64 and
not check for validity soon after. Meaning there can be special
markers like UTIME_NOW and UTIME_OMIT in the nanosecond field which
will make the validity check fail. But, is perfectly normal for the
syscall under consideration.

> * should __sys_recvmmsg() switch to timespec64?

Socket timestamps will be handled in a different series. These could
be done a few ways like adding a new flag to recv syscall variants or
defining new timestamp types. The above call will be changed at that
time.

-Deepa

[toc] | [prev] | [next] | [standalone]

#1675075

On Mon, Jun 26, 2017 at 8:17 PM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> On Sun, Jun 25, 2017 at 7:35 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
>>> The series aims at isolating data conversions of time_t based structures:
>>> struct timespec and struct itimerspec at user space boundaries.
>>> This helps to later change the underlying types to handle y2038 changes
>>> to these.
>>
>> Nice...  A few questions:
>>
>> * what about setitimer(2)?  Right now that's the only remaining user of
>> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().
>
> We do not plan to support these beyond y2038 on 32 bit systems.
> timer_settime() and timer_gettime() are considered to be replacements
> for these, respectively.
>
> There is also going to be a cleanup of timeval/ timespec/ time_t data
> types and apis after the new syscalls are ready.
> At that time I might choose to get rid of these itimerval apis. I'm
> not sure yet.

I see that internally, alarm/getitimer/setitimer all use ktime_t, so
one possible solution would be to push down the use of ktime_t
into the callers and do both the conversion and range check in the
user copy function.

>> * you have two callers of get_compat_itimerspec64(); one is followed by
>> itimerspec64_valid(), another - by its open-coded analogue.  The same
>> goes for get_itimerspec64(); wouldn't it be better to have both check
>> the validity immediately and simply fail with -EINVAL?  Matter of taste,
>> but...
>
> This is what I thought also. And, in fact this is how I had it in one
> of the earlier version of my series.
> But, the utimensat(2) is what I consider provides a counter example of
> why this is a bad idea.
> There is no reason you should not be able to read the itimerspec64 and
> not check for validity soon after. Meaning there can be special
> markers like UTIME_NOW and UTIME_OMIT in the nanosecond field which
> will make the validity check fail. But, is perfectly normal for the
> syscall under consideration.

If this is the only case we find, we could create another get_utimes_arg()
(and compat_get_utimes_arg()) helper just for this one, which then copies
both timespec structures at once and does the appropriate checks.

Another problem might be interfaces that don't have a range check today
with existing users that may pass invalid data and expect it to succeed
(or fail in a particular way). We probably want to add range checks here
in most cases, but having the option of not doing the checking in some
cases may be useful. Maybe I'm just overly cautious here though.

         Arnd

[toc] | [prev] | [next] | [standalone]

#1675286

>>>> The series aims at isolating data conversions of time_t based structures:
>>>> struct timespec and struct itimerspec at user space boundaries.
>>>> This helps to later change the underlying types to handle y2038 changes
>>>> to these.
>>>
>>> Nice...  A few questions:
>>>
>>> * what about setitimer(2)?  Right now that's the only remaining user of
>>> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().
>>
>> We do not plan to support these beyond y2038 on 32 bit systems.
>> timer_settime() and timer_gettime() are considered to be replacements
>> for these, respectively.
>>
>> There is also going to be a cleanup of timeval/ timespec/ time_t data
>> types and apis after the new syscalls are ready.
>> At that time I might choose to get rid of these itimerval apis. I'm
>> not sure yet.
>
> I see that internally, alarm/getitimer/setitimer all use ktime_t, so
> one possible solution would be to push down the use of ktime_t
> into the callers and do both the conversion and range check in the
> user copy function.

Right. This is one way of doing it. I was asking if you guys are okay
with doing this as a cleanup series later or would you like for it to
be part of the current series?

-Deepa

[toc] | [prev] | [next] | [standalone]

#1679864 — Re: [PATCH v3 0/7] Isolate time_t data types for clock/timer syscalls

On Mon, 26 Jun 2017, Arnd Bergmann wrote:
> On Mon, Jun 26, 2017 at 8:17 PM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > On Sun, Jun 25, 2017 at 7:35 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >> On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
> >>> The series aims at isolating data conversions of time_t based structures:
> >>> struct timespec and struct itimerspec at user space boundaries.
> >>> This helps to later change the underlying types to handle y2038 changes
> >>> to these.
> >>
> >> Nice...  A few questions:
> >>
> >> * what about setitimer(2)?  Right now that's the only remaining user of
> >> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().
> >
> > We do not plan to support these beyond y2038 on 32 bit systems.
> > timer_settime() and timer_gettime() are considered to be replacements
> > for these, respectively.
> >
> > There is also going to be a cleanup of timeval/ timespec/ time_t data
> > types and apis after the new syscalls are ready.
> > At that time I might choose to get rid of these itimerval apis. I'm
> > not sure yet.
> 
> I see that internally, alarm/getitimer/setitimer all use ktime_t, so
> one possible solution would be to push down the use of ktime_t
> into the callers and do both the conversion and range check in the
> user copy function.

We still can decide to not support the itimer API with the new y2038 ready
syscalls.

Actually there is no real need to do so because the itimer interfaces are
relative and never absolute. Keeping relative time limited to 68 years from
now should be good enough :)

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1679915

On Mon, Jul 3, 2017 at 12:23 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 26 Jun 2017, Arnd Bergmann wrote:
>> On Mon, Jun 26, 2017 at 8:17 PM, Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>> > On Sun, Jun 25, 2017 at 7:35 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> >> On Sat, Jun 24, 2017 at 11:45:01AM -0700, Deepa Dinamani wrote:
>> >>> The series aims at isolating data conversions of time_t based structures:
>> >>> struct timespec and struct itimerspec at user space boundaries.
>> >>> This helps to later change the underlying types to handle y2038 changes
>> >>> to these.
>> >>
>> >> Nice...  A few questions:
>> >>
>> >> * what about setitimer(2)?  Right now that's the only remaining user of
>> >> get_compat_itimerval(); similar for getitimer(2) and put_compat_itimerval().
>> >
>> > We do not plan to support these beyond y2038 on 32 bit systems.
>> > timer_settime() and timer_gettime() are considered to be replacements
>> > for these, respectively.
>> >
>> > There is also going to be a cleanup of timeval/ timespec/ time_t data
>> > types and apis after the new syscalls are ready.
>> > At that time I might choose to get rid of these itimerval apis. I'm
>> > not sure yet.
>>
>> I see that internally, alarm/getitimer/setitimer all use ktime_t, so
>> one possible solution would be to push down the use of ktime_t
>> into the callers and do both the conversion and range check in the
>> user copy function.
>
> We still can decide to not support the itimer API with the new y2038 ready
> syscalls.
>
> Actually there is no real need to do so because the itimer interfaces are
> relative and never absolute. Keeping relative time limited to 68 years from
> now should be good enough :)

I really want to have all syscalls to use 64-bit time_t for the new
API, otherwise
we get into the really silly state where even for future architectures, glibc
has to convert the 64-bit time_t coming from an application into a 32-bit
timeval to pass it to the kernel, which then converts it back to an internal
type (64-bit time_t or ktime_t).

In case of the itimer interfaces, this is really no question though, as
Deepa said, since glibc can simply implement the new version by calling
timer_create/timer_settime instead of calling the 32-bit setitimer.
timer_settime() needs to take 64-bit arguments anyway because it
can use either relative or absolute arguments.

       Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web