Commit 78c9c4df authored by Thomas Gleixner's avatar Thomas Gleixner
Browse files

posix-timers: Sanitize overrun handling 



The posix timer overrun handling is broken because the forwarding functions
can return a huge number of overruns which does not fit in an int. As a
consequence timer_getoverrun(2) and siginfo::si_overrun can turn into
random number generators.

The k_clock::timer_forward() callbacks return a 64 bit value now. Make
k_itimer::ti_overrun[_last] 64bit as well, so the kernel internal
accounting is correct. 3Remove the temporary (int) casts.

Add a helper function which clamps the overrun value returned to user space
via timer_getoverrun(2) or siginfo::si_overrun limited to a positive value
between 0 and INT_MAX. INT_MAX is an indicator for user space that the
overrun value has been clamped.

Reported-by: default avatarTeam OWL337 <icytxw@gmail.com>
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Acked-by: default avatarJohn Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Link: https://lkml.kernel.org/r/20180626132705.018623573@linutronix.de
parent 6fec64e1

include/linux/posix-timers.h

+2 −2
Original line number Diff line number Diff line
@@ -95,8 +95,8 @@ struct k_itimer { 
	clockid_t		it_clock;

	timer_t			it_id;

	int			it_active;

	int			it_overrun;

	int			it_overrun_last;

	s64			it_overrun;

	s64			it_overrun_last;

	int			it_requeue_pending;

	int			it_sigev_notify;

	ktime_t			it_interval;

kernel/time/posix-cpu-timers.c

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ static void bump_cpu_timer(struct k_itimer *timer, u64 now) 
			continue;



		timer->it.cpu.expires += incr;

		timer->it_overrun += 1 << i;

		timer->it_overrun += 1LL << i;

		delta -= incr;

	}

}

kernel/time/posix-timers.c

+20 −11
Original line number Diff line number Diff line
@@ -283,6 +283,17 @@ static __init int init_posix_timers(void) 
}

__initcall(init_posix_timers);



/*

 * The siginfo si_overrun field and the return value of timer_getoverrun(2)

 * are of type int. Clamp the overrun value to INT_MAX

 */

static inline int timer_overrun_to_int(struct k_itimer *timr, int baseval)

{

	s64 sum = timr->it_overrun_last + (s64)baseval;



	return sum > (s64)INT_MAX ? INT_MAX : (int)sum;

}



static void common_hrtimer_rearm(struct k_itimer *timr)

{

	struct hrtimer *timer = &timr->it.real.timer;
@@ -290,8 +301,7 @@ static void common_hrtimer_rearm(struct k_itimer *timr) 
	if (!timr->it_interval)

		return;



	timr->it_overrun += (unsigned int) hrtimer_forward(timer,

						timer->base->get_time(),

	timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),

					    timr->it_interval);

	hrtimer_restart(timer);

}
@@ -321,10 +331,10 @@ void posixtimer_rearm(struct siginfo *info) 


		timr->it_active = 1;

		timr->it_overrun_last = timr->it_overrun;

		timr->it_overrun = -1;

		timr->it_overrun = -1LL;

		++timr->it_requeue_pending;



		info->si_overrun += timr->it_overrun_last;

		info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);

	}



	unlock_timer(timr, flags);
@@ -418,8 +428,7 @@ 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_overrun += hrtimer_forward(timer, now,

							    timr->it_interval);

			ret = HRTIMER_RESTART;

			++timr->it_requeue_pending;
@@ -524,7 +533,7 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event, 
	new_timer->it_id = (timer_t) new_timer_id;

	new_timer->it_clock = which_clock;

	new_timer->kclock = kc;

	new_timer->it_overrun = -1;

	new_timer->it_overrun = -1LL;



	if (event) {

		rcu_read_lock();
@@ -702,7 +711,7 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting) 
	 * expiry time forward by intervals, so expiry is > now.

	 */

	if (iv && (timr->it_requeue_pending & REQUEUE_PENDING || sig_none))

		timr->it_overrun += (int)kc->timer_forward(timr, now);

		timr->it_overrun += kc->timer_forward(timr, now);



	remaining = kc->timer_remaining(timr, now);

	/* Return 0 only, when the timer is expired and not pending */
@@ -791,7 +800,7 @@ SYSCALL_DEFINE1(timer_getoverrun, timer_t, timer_id) 
	if (!timr)

		return -EINVAL;



	overrun = timr->it_overrun_last;

	overrun = timer_overrun_to_int(timr, 0);

	unlock_timer(timr, flags);



	return overrun;

CRA Git | Maintained and supported by SUSTech CRA and CCSE