diff mbox series

[21/23] y2038: itimer: change implementation to timespec64

Message ID 20191108211323.1806194-12-arnd@arndb.de
State New
Headers show
Series y2038 cleanups | expand

Commit Message

Arnd Bergmann Nov. 8, 2019, 9:12 p.m. UTC 
There is no 64-bit version of getitimer/setitimer since that is not
actually needed. However, the implementation is built around the
deprecated 'struct timeval' type.

Change the code to use timespec64 internally to reduce the dependencies
on timeval and associated helper functions.

Minor adjustments in the code are needed to make the native and compat
version work the same way, and to keep the range check working after
the conversion.

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

---
 include/trace/events/timer.h |   8 +-
 kernel/time/itimer.c         | 158 +++++++++++++++++++++--------------
 2 files changed, 100 insertions(+), 66 deletions(-)

-- 
2.20.0

Comments

Thomas Gleixner Nov. 13, 2019, 10:28 p.m. UTC | #1 
On Fri, 8 Nov 2019, Arnd Bergmann wrote:
>  TRACE_EVENT(itimer_state,

>  

> -	TP_PROTO(int which, const struct itimerval *const value,

> +	TP_PROTO(int which, const struct itimerspec64 *const value,

>  		 unsigned long long expires),

>  

>  	TP_ARGS(which, value, expires),

> @@ -321,12 +321,12 @@ TRACE_EVENT(itimer_state,

>  		__entry->which		= which;

>  		__entry->expires	= expires;

>  		__entry->value_sec	= value->it_value.tv_sec;

> -		__entry->value_usec	= value->it_value.tv_usec;

> +		__entry->value_usec	= value->it_value.tv_nsec / NSEC_PER_USEC;

>  		__entry->interval_sec	= value->it_interval.tv_sec;

> -		__entry->interval_usec	= value->it_interval.tv_usec;

> +		__entry->interval_usec	= value->it_interval.tv_nsec / NSEC_PER_USEC;


Hmm, having a division in a tracepoint is clearly suboptimal.

>  	),

>  

> -	TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",

> +	TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",


We print only 6 digits after the . so that would be even correct w/o a
division. But it probably does not matter much.

> @@ -197,19 +207,13 @@ static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,

>  #define timeval_valid(t) \

>  	(((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))


Hrm, why do we have yet another incarnation of timeval_valid()? Can we
please have only one (the inline version)?

Thanks,

	tglx
Steven Rostedt Nov. 14, 2019, 2:06 a.m. UTC | #2 
On Wed, 13 Nov 2019 23:28:47 +0100 (CET)
Thomas Gleixner <tglx@linutronix.de> wrote:

> On Fri, 8 Nov 2019, Arnd Bergmann wrote:

> >  TRACE_EVENT(itimer_state,

> >  

> > -	TP_PROTO(int which, const struct itimerval *const value,

> > +	TP_PROTO(int which, const struct itimerspec64 *const value,

> >  		 unsigned long long expires),

> >  

> >  	TP_ARGS(which, value, expires),

> > @@ -321,12 +321,12 @@ TRACE_EVENT(itimer_state,

> >  		__entry->which		= which;

> >  		__entry->expires	= expires;

> >  		__entry->value_sec	= value->it_value.tv_sec;

> > -		__entry->value_usec	= value->it_value.tv_usec;

> > +		__entry->value_usec	= value->it_value.tv_nsec / NSEC_PER_USEC;

> >  		__entry->interval_sec	= value->it_interval.tv_sec;

> > -		__entry->interval_usec	= value->it_interval.tv_usec;

> > +		__entry->interval_usec	= value->it_interval.tv_nsec / NSEC_PER_USEC;  

> 

> Hmm, having a division in a tracepoint is clearly suboptimal.


Right, we should move the division into the TP_printk()

		__entry->interval_nsec = alue->it_interval.tv_nsec;

> 

> >  	),

> >  

> > -	TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",

> > +	TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",  

> 

> We print only 6 digits after the . so that would be even correct w/o a

> division. But it probably does not matter much.


Well, we still need the division in the printk, otherwise it will print
more than 6. That's just the minimum and it will print the full number.


		__entry->interval_nsec / NSEC_PER_USEC


-- Steve

> 

> > @@ -197,19 +207,13 @@ static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,

> >  #define timeval_valid(t) \

> >  	(((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))  

> 

> Hrm, why do we have yet another incarnation of timeval_valid()? Can we

> please have only one (the inline version)?

> 

> Thanks,

> 

> 	tglx
Thomas Gleixner Nov. 14, 2019, 10:48 a.m. UTC | #3 
On Wed, 13 Nov 2019, Steven Rostedt wrote:
> On Wed, 13 Nov 2019 23:28:47 +0100 (CET)

> Thomas Gleixner <tglx@linutronix.de> wrote:

> 

> > On Fri, 8 Nov 2019, Arnd Bergmann wrote:

> > >  TRACE_EVENT(itimer_state,

> > >  

> > > -	TP_PROTO(int which, const struct itimerval *const value,

> > > +	TP_PROTO(int which, const struct itimerspec64 *const value,

> > >  		 unsigned long long expires),

> > >  

> > >  	TP_ARGS(which, value, expires),

> > > @@ -321,12 +321,12 @@ TRACE_EVENT(itimer_state,

> > >  		__entry->which		= which;

> > >  		__entry->expires	= expires;

> > >  		__entry->value_sec	= value->it_value.tv_sec;

> > > -		__entry->value_usec	= value->it_value.tv_usec;

> > > +		__entry->value_usec	= value->it_value.tv_nsec / NSEC_PER_USEC;

> > >  		__entry->interval_sec	= value->it_interval.tv_sec;

> > > -		__entry->interval_usec	= value->it_interval.tv_usec;

> > > +		__entry->interval_usec	= value->it_interval.tv_nsec / NSEC_PER_USEC;  

> > 

> > Hmm, having a division in a tracepoint is clearly suboptimal.

> 

> Right, we should move the division into the TP_printk()

> 

> 		__entry->interval_nsec = alue->it_interval.tv_nsec;

> 

> > 

> > >  	),

> > >  

> > > -	TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",

> > > +	TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",  

> > 

> > We print only 6 digits after the . so that would be even correct w/o a

> > division. But it probably does not matter much.

> 

> Well, we still need the division in the printk, otherwise it will print

> more than 6. That's just the minimum and it will print the full number.


That's fine. The print is not really timing critical, the tracepoint very
much so.

Thanks,

	tglx
Arnd Bergmann Nov. 14, 2019, 10:51 a.m. UTC | #4 
On Wed, Nov 13, 2019 at 11:28 PM Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 8 Nov 2019, Arnd Bergmann wrote:

> > @@ -321,12 +321,12 @@ TRACE_EVENT(itimer_state,

> >               __entry->which          = which;

> >               __entry->expires        = expires;

> >               __entry->value_sec      = value->it_value.tv_sec;

> > -             __entry->value_usec     = value->it_value.tv_usec;

> > +             __entry->value_usec     = value->it_value.tv_nsec / NSEC_PER_USEC;

> >               __entry->interval_sec   = value->it_interval.tv_sec;

> > -             __entry->interval_usec  = value->it_interval.tv_usec;

> > +             __entry->interval_usec  = value->it_interval.tv_nsec / NSEC_PER_USEC;

>

> Hmm, having a division in a tracepoint is clearly suboptimal.


Ok, moving it to the TP_printk() as Steven suggested.

> > -     TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",

> > +     TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",

>

> We print only 6 digits after the . so that would be even correct w/o a

> division. But it probably does not matter much.


This is just a cosmetic fix, it can be a separate patch if you care. The idea
is to print the numbers as normal decimal representation, e.g. 0.001000
for a millisecond instead of the nonstandard 0.1000.

> > @@ -197,19 +207,13 @@ static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,

> >  #define timeval_valid(t) \

> >       (((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))

>

> Hrm, why do we have yet another incarnation of timeval_valid()?


No idea, you have to ask the author of commit 7d99b7d634d8 ("[PATCH]
Validate and
sanitze itimer timeval from userspace") ;-)

> Can we please have only one (the inline version)?


I'm removing the inline version in a later patch along with most of the rest of
include/linux/time32.h.

Having the macro version is convenient for this patch, since I'm using it
on two different structures (itimerval/__kernel_old_timeval and
old_itimerval32/old_timeval32), neither of which is the type used in the
inline function.

I could use two local inline functions instead of the macro, or just open
code both call sites if you prefer that.

      Arnd
Arnd Bergmann Nov. 14, 2019, 10:52 a.m. UTC | #5 
On Thu, Nov 14, 2019 at 3:06 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 13 Nov 2019 23:28:47 +0100 (CET) Thomas Gleixner <tglx@linutronix.de> wrote:

> > > -           __entry->value_usec     = value->it_value.tv_usec;

> > > +           __entry->value_usec     = value->it_value.tv_nsec / NSEC_PER_USEC;

> > >             __entry->interval_sec   = value->it_interval.tv_sec;

> > > -           __entry->interval_usec  = value->it_interval.tv_usec;

> > > +           __entry->interval_usec  = value->it_interval.tv_nsec / NSEC_PER_USEC;

> >

> > Hmm, having a division in a tracepoint is clearly suboptimal.

>

> Right, we should move the division into the TP_printk()

>

>                 __entry->interval_nsec = alue->it_interval.tv_nsec;

>


Ok, fixed now, thanks for the suggestion!

     Arnd
Thomas Gleixner Nov. 14, 2019, 10:57 a.m. UTC | #6 
On Thu, 14 Nov 2019, Arnd Bergmann wrote:
> On Wed, Nov 13, 2019 at 11:28 PM Thomas Gleixner <tglx@linutronix.de> wrote:

> > On Fri, 8 Nov 2019, Arnd Bergmann wrote:

> > > @@ -197,19 +207,13 @@ static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,

> > >  #define timeval_valid(t) \

> > >       (((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))

> >

> > Hrm, why do we have yet another incarnation of timeval_valid()?

> 

> No idea, you have to ask the author of commit 7d99b7d634d8 ("[PATCH]

> Validate and

> sanitze itimer timeval from userspace") ;-)


I don't know that guy. :)

> > Can we please have only one (the inline version)?

> 

> I'm removing the inline version in a later patch along with most of the rest of

> include/linux/time32.h.

> 

> Having the macro version is convenient for this patch, since I'm using it

> on two different structures (itimerval/__kernel_old_timeval and

> old_itimerval32/old_timeval32), neither of which is the type used in the

> inline function.


Fair enough.

Thanks,

	tglx
Ben Hutchings Nov. 21, 2019, 4:52 p.m. UTC | #7 
On Fri, 2019-11-08 at 22:12 +0100, Arnd Bergmann wrote:
[...] 
> @@ -292,8 +296,8 @@ static unsigned int alarm_setitimer(unsigned int seconds)

>  	 * We can't return 0 if we have an alarm pending ...  And we'd

>  	 * better return too much than too little anyway

>  	 */

> -	if ((!it_old.it_value.tv_sec && it_old.it_value.tv_usec) ||

> -	      it_old.it_value.tv_usec >= 500000)

> +	if ((!it_old.it_value.tv_sec && it_old.it_value.tv_nsec) ||

> +	      it_old.it_value.tv_nsec >= 500000)

[...]

This is now off by a factor of 1000.  It might be helpful to use
NSEC_PER_SEC / 2 here so no-one has to count the 0 digits.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom
Arnd Bergmann Nov. 25, 2019, 8:26 p.m. UTC | #8 
On Thu, Nov 21, 2019 at 5:52 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
>

> On Fri, 2019-11-08 at 22:12 +0100, Arnd Bergmann wrote:

> [...]

> > @@ -292,8 +296,8 @@ static unsigned int alarm_setitimer(unsigned int seconds)

> >        * We can't return 0 if we have an alarm pending ...  And we'd

> >        * better return too much than too little anyway

> >        */

> > -     if ((!it_old.it_value.tv_sec && it_old.it_value.tv_usec) ||

> > -           it_old.it_value.tv_usec >= 500000)

> > +     if ((!it_old.it_value.tv_sec && it_old.it_value.tv_nsec) ||

> > +           it_old.it_value.tv_nsec >= 500000)

> [...]

>

> This is now off by a factor of 1000.  It might be helpful to use

> NSEC_PER_SEC / 2 here so no-one has to count the 0 digits.


Fixed now, thanks a lot for pointing it out!

        Arnd
diff mbox series

Patch

diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index b7a904825e7d..939cbfc80015 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -303,7 +303,7 @@  DEFINE_EVENT(hrtimer_class, hrtimer_cancel,
  */
 TRACE_EVENT(itimer_state,
 
-	TP_PROTO(int which, const struct itimerval *const value,
+	TP_PROTO(int which, const struct itimerspec64 *const value,
 		 unsigned long long expires),
 
 	TP_ARGS(which, value, expires),
@@ -321,12 +321,12 @@  TRACE_EVENT(itimer_state,
 		__entry->which		= which;
 		__entry->expires	= expires;
 		__entry->value_sec	= value->it_value.tv_sec;
-		__entry->value_usec	= value->it_value.tv_usec;
+		__entry->value_usec	= value->it_value.tv_nsec / NSEC_PER_USEC;
 		__entry->interval_sec	= value->it_interval.tv_sec;
-		__entry->interval_usec	= value->it_interval.tv_usec;
+		__entry->interval_usec	= value->it_interval.tv_nsec / NSEC_PER_USEC;
 	),
 
-	TP_printk("which=%d expires=%llu it_value=%ld.%ld it_interval=%ld.%ld",
+	TP_printk("which=%d expires=%llu it_value=%ld.%06ld it_interval=%ld.%06ld",
 		  __entry->which, __entry->expires,
 		  __entry->value_sec, __entry->value_usec,
 		  __entry->interval_sec, __entry->interval_usec)
diff --git a/kernel/time/itimer.c b/kernel/time/itimer.c
index ce9cd19ce72e..5872db9bd5f7 100644
--- a/kernel/time/itimer.c
+++ b/kernel/time/itimer.c
@@ -26,7 +26,7 @@ 
  * Returns the delta between the expiry time and now, which can be
  * less than zero or 1usec for an pending expired timer
  */
-static struct timeval itimer_get_remtime(struct hrtimer *timer)
+static struct timespec64 itimer_get_remtime(struct hrtimer *timer)
 {
 	ktime_t rem = __hrtimer_get_remaining(timer, true);
 
@@ -41,11 +41,11 @@  static struct timeval itimer_get_remtime(struct hrtimer *timer)
 	} else
 		rem = 0;
 
-	return ktime_to_timeval(rem);
+	return ktime_to_timespec64(rem);
 }
 
 static void get_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
-			   struct itimerval *const value)
+			   struct itimerspec64 *const value)
 {
 	u64 val, interval;
 	struct cpu_itimer *it = &tsk->signal->it[clock_id];
@@ -69,11 +69,11 @@  static void get_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
 
 	spin_unlock_irq(&tsk->sighand->siglock);
 
-	value->it_value = ns_to_timeval(val);
-	value->it_interval = ns_to_timeval(interval);
+	value->it_value = ns_to_timespec64(val);
+	value->it_interval = ns_to_timespec64(interval);
 }
 
-static int do_getitimer(int which, struct itimerval *value)
+static int do_getitimer(int which, struct itimerspec64 *value)
 {
 	struct task_struct *tsk = current;
 
@@ -82,7 +82,7 @@  static int do_getitimer(int which, struct itimerval *value)
 		spin_lock_irq(&tsk->sighand->siglock);
 		value->it_value = itimer_get_remtime(&tsk->signal->real_timer);
 		value->it_interval =
-			ktime_to_timeval(tsk->signal->it_real_incr);
+			ktime_to_timespec64(tsk->signal->it_real_incr);
 		spin_unlock_irq(&tsk->sighand->siglock);
 		break;
 	case ITIMER_VIRTUAL:
@@ -97,17 +97,26 @@  static int do_getitimer(int which, struct itimerval *value)
 	return 0;
 }
 
+static int put_itimerval(struct itimerval __user *o,
+			 const struct itimerspec64 *i)
+{
+	struct itimerval v;
+
+	v.it_interval.tv_sec = i->it_interval.tv_sec;
+	v.it_interval.tv_usec = i->it_interval.tv_nsec / NSEC_PER_USEC;
+	v.it_value.tv_sec = i->it_value.tv_sec;
+	v.it_value.tv_usec = i->it_value.tv_nsec / NSEC_PER_USEC;
+	return copy_to_user(o, &v, sizeof(struct itimerval)) ? -EFAULT : 0;
+}
+
+
 SYSCALL_DEFINE2(getitimer, int, which, struct itimerval __user *, value)
 {
-	int error = -EFAULT;
-	struct itimerval get_buffer;
+	struct itimerspec64 get_buffer;
+	int error = do_getitimer(which, &get_buffer);
 
-	if (value) {
-		error = do_getitimer(which, &get_buffer);
-		if (!error &&
-		    copy_to_user(value, &get_buffer, sizeof(get_buffer)))
-			error = -EFAULT;
-	}
+	if (!error && put_itimerval(value, &get_buffer))
+		error = -EFAULT;
 	return error;
 }
 
@@ -117,24 +126,25 @@  struct old_itimerval32 {
 	struct old_timeval32	it_value;
 };
 
-static int put_old_itimerval32(struct old_itimerval32 __user *o, const struct itimerval *i)
+static int put_old_itimerval32(struct old_itimerval32 __user *o,
+			       const struct itimerspec64 *i)
 {
 	struct old_itimerval32 v32;
 
 	v32.it_interval.tv_sec = i->it_interval.tv_sec;
-	v32.it_interval.tv_usec = i->it_interval.tv_usec;
+	v32.it_interval.tv_usec = i->it_interval.tv_nsec / NSEC_PER_USEC;
 	v32.it_value.tv_sec = i->it_value.tv_sec;
-	v32.it_value.tv_usec = i->it_value.tv_usec;
+	v32.it_value.tv_usec = i->it_value.tv_nsec / NSEC_PER_USEC;
 	return copy_to_user(o, &v32, sizeof(struct old_itimerval32)) ? -EFAULT : 0;
 }
 
 COMPAT_SYSCALL_DEFINE2(getitimer, int, which,
-		       struct old_itimerval32 __user *, it)
+		       struct old_itimerval32 __user *, value)
 {
-	struct itimerval kit;
-	int error = do_getitimer(which, &kit);
+	struct itimerspec64 get_buffer;
+	int error = do_getitimer(which, &get_buffer);
 
-	if (!error && put_old_itimerval32(it, &kit))
+	if (!error && put_old_itimerval32(value, &get_buffer))
 		error = -EFAULT;
 	return error;
 }
@@ -156,8 +166,8 @@  enum hrtimer_restart it_real_fn(struct hrtimer *timer)
 }
 
 static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
-			   const struct itimerval *const value,
-			   struct itimerval *const ovalue)
+			   const struct itimerspec64 *const value,
+			   struct itimerspec64 *const ovalue)
 {
 	u64 oval, nval, ointerval, ninterval;
 	struct cpu_itimer *it = &tsk->signal->it[clock_id];
@@ -166,8 +176,8 @@  static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
 	 * Use the to_ktime conversion because that clamps the maximum
 	 * value to KTIME_MAX and avoid multiplication overflows.
 	 */
-	nval = ktime_to_ns(timeval_to_ktime(value->it_value));
-	ninterval = ktime_to_ns(timeval_to_ktime(value->it_interval));
+	nval = timespec64_to_ns(&value->it_value);
+	ninterval = timespec64_to_ns(&value->it_interval);
 
 	spin_lock_irq(&tsk->sighand->siglock);
 
@@ -186,8 +196,8 @@  static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
 	spin_unlock_irq(&tsk->sighand->siglock);
 
 	if (ovalue) {
-		ovalue->it_value = ns_to_timeval(oval);
-		ovalue->it_interval = ns_to_timeval(ointerval);
+		ovalue->it_value = ns_to_timespec64(oval);
+		ovalue->it_interval = ns_to_timespec64(ointerval);
 	}
 }
 
@@ -197,19 +207,13 @@  static void set_cpu_itimer(struct task_struct *tsk, unsigned int clock_id,
 #define timeval_valid(t) \
 	(((t)->tv_sec >= 0) && (((unsigned long) (t)->tv_usec) < USEC_PER_SEC))
 
-static int do_setitimer(int which, struct itimerval *value, struct itimerval *ovalue)
+static int do_setitimer(int which, struct itimerspec64 *value,
+			struct itimerspec64 *ovalue)
 {
 	struct task_struct *tsk = current;
 	struct hrtimer *timer;
 	ktime_t expires;
 
-	/*
-	 * Validate the timevals in value.
-	 */
-	if (!timeval_valid(&value->it_value) ||
-	    !timeval_valid(&value->it_interval))
-		return -EINVAL;
-
 	switch (which) {
 	case ITIMER_REAL:
 again:
@@ -218,7 +222,7 @@  static int do_setitimer(int which, struct itimerval *value, struct itimerval *ov
 		if (ovalue) {
 			ovalue->it_value = itimer_get_remtime(timer);
 			ovalue->it_interval
-				= ktime_to_timeval(tsk->signal->it_real_incr);
+				= ktime_to_timespec64(tsk->signal->it_real_incr);
 		}
 		/* We are sharing ->siglock with it_real_fn() */
 		if (hrtimer_try_to_cancel(timer) < 0) {
@@ -226,10 +230,10 @@  static int do_setitimer(int which, struct itimerval *value, struct itimerval *ov
 			hrtimer_cancel_wait_running(timer);
 			goto again;
 		}
-		expires = timeval_to_ktime(value->it_value);
+		expires = timespec64_to_ktime(value->it_value);
 		if (expires != 0) {
 			tsk->signal->it_real_incr =
-				timeval_to_ktime(value->it_interval);
+				timespec64_to_ktime(value->it_interval);
 			hrtimer_start(timer, expires, HRTIMER_MODE_REL);
 		} else
 			tsk->signal->it_real_incr = 0;
@@ -252,7 +256,7 @@  static int do_setitimer(int which, struct itimerval *value, struct itimerval *ov
 #ifdef CONFIG_SECURITY_SELINUX
 void clear_itimer(void)
 {
-	struct itimerval v = {};
+	struct itimerspec64 v = {};
 	int i;
 
 	for (i = 0; i < 3; i++)
@@ -276,15 +280,15 @@  void clear_itimer(void)
  */
 static unsigned int alarm_setitimer(unsigned int seconds)
 {
-	struct itimerval it_new, it_old;
+	struct itimerspec64 it_new, it_old;
 
 #if BITS_PER_LONG < 64
 	if (seconds > INT_MAX)
 		seconds = INT_MAX;
 #endif
 	it_new.it_value.tv_sec = seconds;
-	it_new.it_value.tv_usec = 0;
-	it_new.it_interval.tv_sec = it_new.it_interval.tv_usec = 0;
+	it_new.it_value.tv_nsec = 0;
+	it_new.it_interval.tv_sec = it_new.it_interval.tv_nsec = 0;
 
 	do_setitimer(ITIMER_REAL, &it_new, &it_old);
 
@@ -292,8 +296,8 @@  static unsigned int alarm_setitimer(unsigned int seconds)
 	 * We can't return 0 if we have an alarm pending ...  And we'd
 	 * better return too much than too little anyway
 	 */
-	if ((!it_old.it_value.tv_sec && it_old.it_value.tv_usec) ||
-	      it_old.it_value.tv_usec >= 500000)
+	if ((!it_old.it_value.tv_sec && it_old.it_value.tv_nsec) ||
+	      it_old.it_value.tv_nsec >= 500000)
 		it_old.it_value.tv_sec++;
 
 	return it_old.it_value.tv_sec;
@@ -310,15 +314,35 @@  SYSCALL_DEFINE1(alarm, unsigned int, seconds)
 
 #endif
 
+static int get_itimerval(struct itimerspec64 *o, const struct itimerval __user *i)
+{
+	struct itimerval v;
+
+	if (copy_from_user(&v, i, sizeof(struct itimerval)))
+		return -EFAULT;
+
+	/* Validate the timevals in value. */
+	if (!timeval_valid(&v.it_value) ||
+	    !timeval_valid(&v.it_interval))
+		return -EINVAL;
+
+	o->it_interval.tv_sec = v.it_interval.tv_sec;
+	o->it_interval.tv_nsec = v.it_interval.tv_usec * NSEC_PER_USEC;
+	o->it_value.tv_sec = v.it_value.tv_sec;
+	o->it_value.tv_nsec = v.it_value.tv_usec * NSEC_PER_USEC;
+	return 0;
+}
+
 SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
 		struct itimerval __user *, ovalue)
 {
-	struct itimerval set_buffer, get_buffer;
+	struct itimerspec64 set_buffer, get_buffer;
 	int error;
 
 	if (value) {
-		if(copy_from_user(&set_buffer, value, sizeof(set_buffer)))
-			return -EFAULT;
+		error = get_itimerval(&set_buffer, value);
+		if (error)
+			return error;
 	} else {
 		memset(&set_buffer, 0, sizeof(set_buffer));
 		printk_once(KERN_WARNING "%s calls setitimer() with new_value NULL pointer."
@@ -330,43 +354,53 @@  SYSCALL_DEFINE3(setitimer, int, which, struct itimerval __user *, value,
 	if (error || !ovalue)
 		return error;
 
-	if (copy_to_user(ovalue, &get_buffer, sizeof(get_buffer)))
+	if (put_itimerval(ovalue, &get_buffer))
 		return -EFAULT;
 	return 0;
 }
 
 #if defined(CONFIG_COMPAT) || defined(CONFIG_ALPHA)
-static int get_old_itimerval32(struct itimerval *o, const struct old_itimerval32 __user *i)
+static int get_old_itimerval32(struct itimerspec64 *o, const struct old_itimerval32 __user *i)
 {
 	struct old_itimerval32 v32;
 
 	if (copy_from_user(&v32, i, sizeof(struct old_itimerval32)))
 		return -EFAULT;
+
+	/* Validate the timevals in value.  */
+	if (!timeval_valid(&v32.it_value) ||
+	    !timeval_valid(&v32.it_interval))
+		return -EINVAL;
+
 	o->it_interval.tv_sec = v32.it_interval.tv_sec;
-	o->it_interval.tv_usec = v32.it_interval.tv_usec;
+	o->it_interval.tv_nsec = v32.it_interval.tv_usec * NSEC_PER_USEC;
 	o->it_value.tv_sec = v32.it_value.tv_sec;
-	o->it_value.tv_usec = v32.it_value.tv_usec;
+	o->it_value.tv_nsec = v32.it_value.tv_usec * NSEC_PER_USEC;
 	return 0;
 }
 
 COMPAT_SYSCALL_DEFINE3(setitimer, int, which,
-		       struct old_itimerval32 __user *, in,
-		       struct old_itimerval32 __user *, out)
+		       struct old_itimerval32 __user *, value,
+		       struct old_itimerval32 __user *, ovalue)
 {
-	struct itimerval kin, kout;
+	struct itimerspec64 set_buffer, get_buffer;
 	int error;
 
-	if (in) {
-		if (get_old_itimerval32(&kin, in))
-			return -EFAULT;
+	if (value) {
+		error = get_old_itimerval32(&set_buffer, value);
+		if (error)
+			return error;
 	} else {
-		memset(&kin, 0, sizeof(kin));
+		memset(&set_buffer, 0, sizeof(set_buffer));
+		printk_once(KERN_WARNING "%s calls setitimer() with new_value NULL pointer."
+			    " Misfeature support will be removed\n",
+			    current->comm);
 	}
 
-	error = do_setitimer(which, &kin, out ? &kout : NULL);
-	if (error || !out)
+	error = do_setitimer(which, &set_buffer, ovalue ? &get_buffer : NULL);
+	if (error || !ovalue)
 		return error;
-	if (put_old_itimerval32(out, &kout))
+	if (put_old_itimerval32(ovalue, &get_buffer))
 		return -EFAULT;
 	return 0;
 }