hrtimer: check relative timeouts for overflow

Various user space callers ask for relative timeouts. While we fixed that overflow issue in hrtimer_start(), the sites which convert relative user space values to absolute timeouts themself were uncovered. Instead of putting overflow checks into each place add a function which does the sanity checking and convert all affected callers to use it. Thanks to Frans Pop, who reported the problem and tested the fixes. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Ingo Molnar <mingo@elte.hu> Tested-by: Frans Pop <elendil@planet.nl>

hrtimer: check relative timeouts for overflow
Various user space callers ask for relative timeouts. While we fixed that overflow issue in hrtimer_start(), the sites which convert relative user space values to absolute timeouts themself were uncovered. Instead of putting overflow checks into each place add a function which does the sanity checking and convert all affected callers to use it. Thanks to Frans Pop, who reported the problem and tested the fixes. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Ingo Molnar <mingo@elte.hu> Tested-by: Frans Pop <elendil@planet.nl>
Thomas Gleixner
1 parent e760e716d4
Showing 5 changed files with 29 additions and 22 deletions Side-by-side Diff
include/linux/ktime.h
kernel/futex.c
kernel/futex_compat.c
kernel/hrtimer.c
kernel/posix-timers.c
@@ -310,6 +310,8 @@
 	return ktime_sub_ns(kt, usec * 1000);
 }
  
+extern ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs);
+
 /*
  * The resolution of the clocks. The resolution value is returned in
  * the clock_getres() system call to give application programmers an
@@ -2116,7 +2116,7 @@
  
 		t = timespec_to_ktime(ts);
 		if (cmd == FUTEX_WAIT)
-			t = ktime_add(ktime_get(), t);
+			t = ktime_add_safe(ktime_get(), t);
 		tp = &t;
 	}
 	/*
@@ -176,7 +176,7 @@
  
 		t = timespec_to_ktime(ts);
 		if (cmd == FUTEX_WAIT)
-			t = ktime_add(ktime_get(), t);
+			t = ktime_add_safe(ktime_get(), t);
 		tp = &t;
 	}
 	if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE)
@@ -326,6 +326,23 @@
 #endif /* BITS_PER_LONG >= 64 */
  
 /*
+ * Add two ktime values and do a safety check for overflow:
+ */
+ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
+{
+	ktime_t res = ktime_add(lhs, rhs);
+
+	/*
+	 * We use KTIME_SEC_MAX here, the maximum timeout which we can
+	 * return to user space in a timespec:
+	 */
+	if (res.tv64 < 0 || res.tv64 < lhs.tv64 || res.tv64 < rhs.tv64)
+		res = ktime_set(KTIME_SEC_MAX, 0);
+
+	return res;
+}
+
+/*
  * Check, whether the timer is on the callback pending list
  */
 static inline int hrtimer_cb_pending(const struct hrtimer *timer)
@@ -682,13 +699,7 @@
 		 */
 		orun++;
 	}
-	timer->expires = ktime_add(timer->expires, interval);
-	/*
-	 * Make sure, that the result did not wrap with a very large
-	 * interval.
-	 */
-	if (timer->expires.tv64 < 0)
-		timer->expires = ktime_set(KTIME_SEC_MAX, 0);
+	timer->expires = ktime_add_safe(timer->expires, interval);
  
 	return orun;
 }
@@ -839,7 +850,7 @@
 	new_base = switch_hrtimer_base(timer, base);
  
 	if (mode == HRTIMER_MODE_REL) {
-		tim = ktime_add(tim, new_base->get_time());
+		tim = ktime_add_safe(tim, new_base->get_time());
 		/*
 		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
 		 * to signal that they simply return xtime in
  
@@ -848,16 +859,8 @@
 		 * timeouts. This will go away with the GTOD framework.
 		 */
 #ifdef CONFIG_TIME_LOW_RES
-		tim = ktime_add(tim, base->resolution);
+		tim = ktime_add_safe(tim, base->resolution);
 #endif
-		/*
-		 * Careful here: User space might have asked for a
-		 * very long sleep, so the add above might result in a
-		 * negative number, which enqueues the timer in front
-		 * of the queue.
-		 */
-		if (tim.tv64 < 0)
-			tim.tv64 = KTIME_MAX;
 	}
 	timer->expires = tim;
  
@@ -767,9 +767,11 @@
 	/* SIGEV_NONE timers are not queued ! See common_timer_get */
 	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
 		/* Setup correct expiry time for relative timers */
-		if (mode == HRTIMER_MODE_REL)
-			timer->expires = ktime_add(timer->expires,
-						   timer->base->get_time());
+		if (mode == HRTIMER_MODE_REL) {
+			timer->expires =
+				ktime_add_safe(timer->expires,
+					       timer->base->get_time());
+		}
 		return 0;
 	}
...	...	@@ -310,6 +310,8 @@
310	310	return ktime_sub_ns(kt, usec * 1000);
311	311	}
312	312
	313	+extern ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs);
	314	+
313	315	/*
314	316	* The resolution of the clocks. The resolution value is returned in
315	317	* the clock_getres() system call to give application programmers an
...	...	@@ -2116,7 +2116,7 @@
2116	2116
2117	2117	t = timespec_to_ktime(ts);
2118	2118	if (cmd == FUTEX_WAIT)
2119		- t = ktime_add(ktime_get(), t);
	2119	+ t = ktime_add_safe(ktime_get(), t);
2120	2120	tp = &t;
2121	2121	}
2122	2122	/*
...	...	@@ -176,7 +176,7 @@
176	176
177	177	t = timespec_to_ktime(ts);
178	178	if (cmd == FUTEX_WAIT)
179		- t = ktime_add(ktime_get(), t);
	179	+ t = ktime_add_safe(ktime_get(), t);
180	180	tp = &t;
181	181	}
182	182	if (cmd == FUTEX_REQUEUE \|\| cmd == FUTEX_CMP_REQUEUE)
...	...	@@ -326,6 +326,23 @@
326	326	#endif /* BITS_PER_LONG >= 64 */
327	327
328	328	/*
	329	+ * Add two ktime values and do a safety check for overflow:
	330	+ */
	331	+ktime_t ktime_add_safe(const ktime_t lhs, const ktime_t rhs)
	332	+{
	333	+ ktime_t res = ktime_add(lhs, rhs);
	334	+
	335	+ /*
	336	+ * We use KTIME_SEC_MAX here, the maximum timeout which we can
	337	+ * return to user space in a timespec:
	338	+ */
	339	+ if (res.tv64 < 0 \|\| res.tv64 < lhs.tv64 \|\| res.tv64 < rhs.tv64)
	340	+ res = ktime_set(KTIME_SEC_MAX, 0);
	341	+
	342	+ return res;
	343	+}
	344	+
	345	+/*
329	346	* Check, whether the timer is on the callback pending list
330	347	*/
331	348	static inline int hrtimer_cb_pending(const struct hrtimer *timer)
...	...	@@ -682,13 +699,7 @@
682	699	*/
683	700	orun++;
684	701	}
685		- timer->expires = ktime_add(timer->expires, interval);
686		- /*
687		- * Make sure, that the result did not wrap with a very large
688		- * interval.
689		- */
690		- if (timer->expires.tv64 < 0)
691		- timer->expires = ktime_set(KTIME_SEC_MAX, 0);
	702	+ timer->expires = ktime_add_safe(timer->expires, interval);
692	703
693	704	return orun;
694	705	}
...	...	@@ -839,7 +850,7 @@
839	850	new_base = switch_hrtimer_base(timer, base);
840	851
841	852	if (mode == HRTIMER_MODE_REL) {
842		- tim = ktime_add(tim, new_base->get_time());
	853	+ tim = ktime_add_safe(tim, new_base->get_time());
843	854	/*
844	855	* CONFIG_TIME_LOW_RES is a temporary way for architectures
845	856	* to signal that they simply return xtime in
846	857
...	...	@@ -848,16 +859,8 @@
848	859	* timeouts. This will go away with the GTOD framework.
849	860	*/
850	861	#ifdef CONFIG_TIME_LOW_RES
851		- tim = ktime_add(tim, base->resolution);
	862	+ tim = ktime_add_safe(tim, base->resolution);
852	863	#endif
853		- /*
854		- * Careful here: User space might have asked for a
855		- * very long sleep, so the add above might result in a
856		- * negative number, which enqueues the timer in front
857		- * of the queue.
858		- */
859		- if (tim.tv64 < 0)
860		- tim.tv64 = KTIME_MAX;
861	864	}
862	865	timer->expires = tim;
863	866
...	...	@@ -767,9 +767,11 @@
767	767	/* SIGEV_NONE timers are not queued ! See common_timer_get */
768	768	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
769	769	/* Setup correct expiry time for relative timers */
770		- if (mode == HRTIMER_MODE_REL)
771		- timer->expires = ktime_add(timer->expires,
772		- timer->base->get_time());
	770	+ if (mode == HRTIMER_MODE_REL) {
	771	+ timer->expires =
	772	+ ktime_add_safe(timer->expires,
	773	+ timer->base->get_time());
	774	+ }
773	775	return 0;
774	776	}
775	777