[PATCH 0/2] posix-timers: Prevents overrun counter overflow, adds DELAYTIMER_MAX

[PATCH 0/2] posix-timers: Prevents overrun counter overflow, adds DELAYTIMER_MAX

[Daniel Church]

If a timer overruns too many times before a call to timer_getoverrun the
overrun count can overflow and go negative.  POSIX.1-2001 specification
of timer_getoverrun() supports constant DELAYTIMER_MAX which prevents
overflow and caps overrun count.  Adds delaytimer_max value, caps
overrun count, and exposes value to userland via
/proc/sys/kernel/delaytimer_max.

Resolves https://bugzilla.kernel.org/show_bug.cgi?id=12665 - POSIX timers API does not support DELAYTIMER_MAX

[PATCH 1/2] posix-timers: Prevents overrun counter overflow

[Daniel Church]

If a timer overruns too many times before a call to timer_getoverrun the
overrun count can overflow and go negative.  Adds delaytimer_max value
to cap overrun count and prevent overflow.

Signed-off-by: Daniel Church <dchurch@andplus.com>
---
 include/linux/posix-timers.h |  3 +++
 kernel/time/posix-timers.c   | 42 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd..dc8a1e7 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -138,4 +138,7 @@ long clock_nanosleep_restart(struct restart_block *restart_block);
 
 void update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new);
 
+#define DELAYTIMER_MAX_DEFAULT 1000000
+extern int delaytimer_max;
+
 #endif
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 31ea01f..bdf0c0f 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -68,6 +68,8 @@ static struct kmem_cache *posix_timers_cache;
 static DEFINE_HASHTABLE(posix_timers_hashtable, 9);
 static DEFINE_SPINLOCK(hash_lock);
 
+int delaytimer_max = DELAYTIMER_MAX_DEFAULT;
+
 /*
  * we assume that the new SIGEV_THREAD_ID shares no bits with the other
  * SIGEV values.  Here we put out an error if this assumption fails.
@@ -350,14 +352,22 @@ __initcall(init_posix_timers);
 
 static void schedule_next_timer(struct k_itimer *timr)
 {
+	unsigned int overruns;
 	struct hrtimer *timer = &timr->it.real.timer;
 
 	if (timr->it.real.interval.tv64 == 0)
 		return;
 
-	timr->it_overrun += (unsigned int) hrtimer_forward(timer,
-						timer->base->get_time(),
-						timr->it.real.interval);
+	overruns = (unsigned int) hrtimer_forward(timer,
+					timer->base->get_time(),
+					timr->it.real.interval);
+	if (overruns >= delaytimer_max ||
+		(timr->it_overrun >= 0 &&
+		 timr->it_overrun >= delaytimer_max - overruns)) {
+		timr->it_overrun = delaytimer_max;
+	} else {
+		timr->it_overrun += overruns;
+	}
 
 	timr->it_overrun_last = timr->it_overrun;
 	timr->it_overrun = -1;
@@ -436,6 +446,7 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
 {
 	struct k_itimer *timr;
 	unsigned long flags;
+	unsigned int overruns;
 	int si_private = 0;
 	enum hrtimer_restart ret = HRTIMER_NORESTART;
 
@@ -484,9 +495,15 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
 					now = ktime_add(now, kj);
 			}
 #endif
-			timr->it_overrun += (unsigned int)
-				hrtimer_forward(timer, now,
-						timr->it.real.interval);
+			overruns = (unsigned int) hrtimer_forward(timer, now,
+								 timr->it.real.interval);
+			if (overruns >= delaytimer_max ||
+				(timr->it_overrun >= 0 &&
+				 timr->it_overrun >= delaytimer_max - overruns)) {
+				timr->it_overrun = delaytimer_max;
+			} else {
+				timr->it_overrun += overruns;
+			}
 			ret = HRTIMER_RESTART;
 			++timr->it_requeue_pending;
 		}
@@ -729,6 +746,7 @@ static void
 common_timer_get(struct k_itimer *timr, struct itimerspec *cur_setting)
 {
 	ktime_t now, remaining, iv;
+	unsigned int overruns;
 	struct hrtimer *timer = &timr->it.real.timer;
 
 	memset(cur_setting, 0, sizeof(struct itimerspec));
@@ -750,8 +768,16 @@ common_timer_get(struct k_itimer *timr, struct itimerspec *cur_setting)
 	 * expiry is > now.
 	 */
 	if (iv.tv64 && (timr->it_requeue_pending & REQUEUE_PENDING ||
-	    (timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
-		timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
+	    (timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
+		overruns = (unsigned int) hrtimer_forward(timer, now, iv);
+		if (overruns >= delaytimer_max ||
+			(timr->it_overrun >= 0 &&
+			 timr->it_overrun >= delaytimer_max - overruns)) {
+			timr->it_overrun = delaytimer_max;
+		} else {
+			timr->it_overrun += overruns;
+		}
+	}
 
 	remaining = ktime_sub(hrtimer_get_expires(timer), now);
 	/* Return 0 only, when the timer is expired and not pending */
-- 
1.9.1

[PATCH 2/2] posix-timers: Exposes DELAYTIMER_MAX constant used to govern overruns

[Daniel Church]

POSIX.1-2001 specification of timer_getoverrun() supports constant
DELAYTIMER_MAX which prevents overflow and caps overrun count.  Exposes
delaytimer_max value to userland via /proc/sys/kernel/delaytimer_max such
that GLIBC can support DELAYTIMER_MAX constant.

Signed-off-by: Daniel Church <dchurch@andplus.com>
---
 kernel/sysctl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 137c7f6..b283808 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -63,6 +63,7 @@
 #include <linux/binfmts.h>
 #include <linux/sched/sysctl.h>
 #include <linux/kexec.h>
+#include <linux/posix-timers.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -761,6 +762,13 @@ static struct ctl_table kern_table[] = {
 	},
 #endif
 	{
+		.procname	= "delaytimer_max",
+		.data		= &delaytimer_max,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec,
+	},
+	{
 		.procname	= "pid_max",
 		.data		= &pid_max,
 		.maxlen		= sizeof (int),
-- 
1.9.1

Re: [PATCH 1/2] posix-timers: Prevents overrun counter overflow

[Thomas Gleixner]

On Fri, 23 Jan 2015, Daniel Church wrote:
> +	overruns = (unsigned int) hrtimer_forward(timer,
> +					timer->base->get_time(),
> +					timr->it.real.interval);
> +	if (overruns >= delaytimer_max ||
> +		(timr->it_overrun >= 0 &&
> +		 timr->it_overrun >= delaytimer_max - overruns)) {
> +		timr->it_overrun = delaytimer_max;
> +	} else {
> +		timr->it_overrun += overruns;
> +	}

We certainly do not add the same logic 3 times via copy and
paste. Please make that a proper helper function.

Thanks,

	tglx