Skip to content

Commit 78c9c4d

Browse files
committed
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: Team OWL337 <icytxw@gmail.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: John 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
1 parent 6fec64e commit 78c9c4d

File tree

3 files changed

+23
-14
lines changed

3 files changed

+23
-14
lines changed

Diff for: include/linux/posix-timers.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ struct k_itimer {
9595
clockid_t it_clock;
9696
timer_t it_id;
9797
int it_active;
98-
int it_overrun;
99-
int it_overrun_last;
98+
s64 it_overrun;
99+
s64 it_overrun_last;
100100
int it_requeue_pending;
101101
int it_sigev_notify;
102102
ktime_t it_interval;

Diff for: kernel/time/posix-cpu-timers.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ static void bump_cpu_timer(struct k_itimer *timer, u64 now)
8585
continue;
8686

8787
timer->it.cpu.expires += incr;
88-
timer->it_overrun += 1 << i;
88+
timer->it_overrun += 1LL << i;
8989
delta -= incr;
9090
}
9191
}

Diff for: kernel/time/posix-timers.c

+20-11
Original file line numberDiff line numberDiff line change
@@ -283,16 +283,26 @@ static __init int init_posix_timers(void)
283283
}
284284
__initcall(init_posix_timers);
285285

286+
/*
287+
* The siginfo si_overrun field and the return value of timer_getoverrun(2)
288+
* are of type int. Clamp the overrun value to INT_MAX
289+
*/
290+
static inline int timer_overrun_to_int(struct k_itimer *timr, int baseval)
291+
{
292+
s64 sum = timr->it_overrun_last + (s64)baseval;
293+
294+
return sum > (s64)INT_MAX ? INT_MAX : (int)sum;
295+
}
296+
286297
static void common_hrtimer_rearm(struct k_itimer *timr)
287298
{
288299
struct hrtimer *timer = &timr->it.real.timer;
289300

290301
if (!timr->it_interval)
291302
return;
292303

293-
timr->it_overrun += (unsigned int) hrtimer_forward(timer,
294-
timer->base->get_time(),
295-
timr->it_interval);
304+
timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),
305+
timr->it_interval);
296306
hrtimer_restart(timer);
297307
}
298308

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

322332
timr->it_active = 1;
323333
timr->it_overrun_last = timr->it_overrun;
324-
timr->it_overrun = -1;
334+
timr->it_overrun = -1LL;
325335
++timr->it_requeue_pending;
326336

327-
info->si_overrun += timr->it_overrun_last;
337+
info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);
328338
}
329339

330340
unlock_timer(timr, flags);
@@ -418,9 +428,8 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer)
418428
now = ktime_add(now, kj);
419429
}
420430
#endif
421-
timr->it_overrun += (unsigned int)
422-
hrtimer_forward(timer, now,
423-
timr->it_interval);
431+
timr->it_overrun += hrtimer_forward(timer, now,
432+
timr->it_interval);
424433
ret = HRTIMER_RESTART;
425434
++timr->it_requeue_pending;
426435
timr->it_active = 1;
@@ -524,7 +533,7 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event,
524533
new_timer->it_id = (timer_t) new_timer_id;
525534
new_timer->it_clock = which_clock;
526535
new_timer->kclock = kc;
527-
new_timer->it_overrun = -1;
536+
new_timer->it_overrun = -1LL;
528537

529538
if (event) {
530539
rcu_read_lock();
@@ -702,7 +711,7 @@ void common_timer_get(struct k_itimer *timr, struct itimerspec64 *cur_setting)
702711
* expiry time forward by intervals, so expiry is > now.
703712
*/
704713
if (iv && (timr->it_requeue_pending & REQUEUE_PENDING || sig_none))
705-
timr->it_overrun += (int)kc->timer_forward(timr, now);
714+
timr->it_overrun += kc->timer_forward(timr, now);
706715

707716
remaining = kc->timer_remaining(timr, now);
708717
/* Return 0 only, when the timer is expired and not pending */
@@ -791,7 +800,7 @@ SYSCALL_DEFINE1(timer_getoverrun, timer_t, timer_id)
791800
if (!timr)
792801
return -EINVAL;
793802

794-
overrun = timr->it_overrun_last;
803+
overrun = timer_overrun_to_int(timr, 0);
795804
unlock_timer(timr, flags);
796805

797806
return overrun;

0 commit comments

Comments
 (0)