From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 78c9c4dfbf8c04883941445a195276bb4bb92c76 ]
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 [florian: Make patch apply to v4.9.135] Signed-off-by: Florian Fainelli f.fainelli@gmail.com --- Thomas, can you review for correctness? Thanks!
include/linux/posix-timers.h | 4 ++-- kernel/time/posix-cpu-timers.c | 2 +- kernel/time/posix-timers.c | 29 +++++++++++++++++++---------- 3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 62d44c176071..e4b8678183f5 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -65,8 +65,8 @@ struct k_itimer { spinlock_t it_lock; clockid_t it_clock; /* which timer type */ timer_t it_id; /* timer id */ - int it_overrun; /* overrun on pending signal */ - int it_overrun_last; /* overrun on last delivered signal */ + s64 it_overrun; /* overrun on pending signal */ + s64 it_overrun_last; /* overrun on last delivered signal */ int it_requeue_pending; /* waiting to requeue this timer */ #define REQUEUE_PENDING 1 int it_sigev_notify; /* notify word of sigevent struct */ diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 39008d78927a..21a27bb73587 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -103,7 +103,7 @@ static void bump_cpu_timer(struct k_itimer *timer, continue;
timer->it.cpu.expires += incr; - timer->it_overrun += 1 << i; + timer->it_overrun += 1LL << i; delta -= incr; } } diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index fc7c37ad90a0..0e6ed2e7d066 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -355,6 +355,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 schedule_next_timer(struct k_itimer *timr) { struct hrtimer *timer = &timr->it.real.timer; @@ -362,12 +373,11 @@ static void schedule_next_timer(struct k_itimer *timr) if (timr->it.real.interval.tv64 == 0) return;
- timr->it_overrun += (unsigned int) hrtimer_forward(timer, - timer->base->get_time(), - timr->it.real.interval); + timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(), + timr->it.real.interval);
timr->it_overrun_last = timr->it_overrun; - timr->it_overrun = -1; + timr->it_overrun = -1LL; ++timr->it_requeue_pending; hrtimer_restart(timer); } @@ -396,7 +406,7 @@ void do_schedule_next_timer(struct siginfo *info) else schedule_next_timer(timr);
- info->si_overrun += timr->it_overrun_last; + info->si_overrun = timer_overrun_to_int(timr, info->si_overrun); }
if (timr) @@ -491,8 +501,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.real.interval); ret = HRTIMER_RESTART; ++timr->it_requeue_pending; @@ -633,7 +642,7 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock, it_id_set = IT_ID_SET; new_timer->it_id = (timer_t) new_timer_id; new_timer->it_clock = which_clock; - new_timer->it_overrun = -1; + new_timer->it_overrun = -1LL;
if (timer_event_spec) { if (copy_from_user(&event, timer_event_spec, sizeof (event))) { @@ -762,7 +771,7 @@ common_timer_get(struct k_itimer *timr, struct itimerspec *cur_setting) */ if (iv.tv64 && (timr->it_requeue_pending & REQUEUE_PENDING || timr->it_sigev_notify == SIGEV_NONE)) - timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv); + timr->it_overrun += hrtimer_forward(timer, now, iv);
remaining = __hrtimer_expires_remaining_adjusted(timer, now); /* Return 0 only, when the timer is expired and not pending */ @@ -824,7 +833,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;
On 11/1/18 1:02 PM, Florian Fainelli wrote:
From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 78c9c4dfbf8c04883941445a195276bb4bb92c76 ]
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 [florian: Make patch apply to v4.9.135] Signed-off-by: Florian Fainelli f.fainelli@gmail.com
Thomas, can you review for correctness? Thanks!
Thomas, John, does that look like a reasonable backport for 4.9?
include/linux/posix-timers.h | 4 ++-- kernel/time/posix-cpu-timers.c | 2 +- kernel/time/posix-timers.c | 29 +++++++++++++++++++---------- 3 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h index 62d44c176071..e4b8678183f5 100644 --- a/include/linux/posix-timers.h +++ b/include/linux/posix-timers.h @@ -65,8 +65,8 @@ struct k_itimer { spinlock_t it_lock; clockid_t it_clock; /* which timer type */ timer_t it_id; /* timer id */
- int it_overrun; /* overrun on pending signal */
- int it_overrun_last; /* overrun on last delivered signal */
- s64 it_overrun; /* overrun on pending signal */
- s64 it_overrun_last; /* overrun on last delivered signal */ int it_requeue_pending; /* waiting to requeue this timer */
#define REQUEUE_PENDING 1 int it_sigev_notify; /* notify word of sigevent struct */ diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c index 39008d78927a..21a27bb73587 100644 --- a/kernel/time/posix-cpu-timers.c +++ b/kernel/time/posix-cpu-timers.c @@ -103,7 +103,7 @@ static void bump_cpu_timer(struct k_itimer *timer, continue; timer->it.cpu.expires += incr;
timer->it_overrun += 1 << i;
delta -= incr; }timer->it_overrun += 1LL << i;
} diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c index fc7c37ad90a0..0e6ed2e7d066 100644 --- a/kernel/time/posix-timers.c +++ b/kernel/time/posix-timers.c @@ -355,6 +355,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 schedule_next_timer(struct k_itimer *timr) { struct hrtimer *timer = &timr->it.real.timer; @@ -362,12 +373,11 @@ static void schedule_next_timer(struct k_itimer *timr) if (timr->it.real.interval.tv64 == 0) return;
- timr->it_overrun += (unsigned int) hrtimer_forward(timer,
timer->base->get_time(),
timr->it.real.interval);
- timr->it_overrun += hrtimer_forward(timer, timer->base->get_time(),
timr->it.real.interval);
timr->it_overrun_last = timr->it_overrun;
- timr->it_overrun = -1;
- timr->it_overrun = -1LL; ++timr->it_requeue_pending; hrtimer_restart(timer);
} @@ -396,7 +406,7 @@ void do_schedule_next_timer(struct siginfo *info) else schedule_next_timer(timr);
info->si_overrun += timr->it_overrun_last;
}info->si_overrun = timer_overrun_to_int(timr, info->si_overrun);
if (timr) @@ -491,8 +501,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.real.interval); ret = HRTIMER_RESTART; ++timr->it_requeue_pending;
@@ -633,7 +642,7 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock, it_id_set = IT_ID_SET; new_timer->it_id = (timer_t) new_timer_id; new_timer->it_clock = which_clock;
- new_timer->it_overrun = -1;
- new_timer->it_overrun = -1LL;
if (timer_event_spec) { if (copy_from_user(&event, timer_event_spec, sizeof (event))) { @@ -762,7 +771,7 @@ common_timer_get(struct k_itimer *timr, struct itimerspec *cur_setting) */ if (iv.tv64 && (timr->it_requeue_pending & REQUEUE_PENDING || timr->it_sigev_notify == SIGEV_NONE))
timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
timr->it_overrun += hrtimer_forward(timer, now, iv);
remaining = __hrtimer_expires_remaining_adjusted(timer, now); /* Return 0 only, when the timer is expired and not pending */ @@ -824,7 +833,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;
Florian,
On Wed, 7 Nov 2018, Florian Fainelli wrote:
On 11/1/18 1:02 PM, Florian Fainelli wrote:
From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 78c9c4dfbf8c04883941445a195276bb4bb92c76 ]
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 [florian: Make patch apply to v4.9.135] Signed-off-by: Florian Fainelli f.fainelli@gmail.com
Thomas, can you review for correctness? Thanks!
Thomas, John, does that look like a reasonable backport for 4.9?
Looks correct.
Thanks,
tglx
On Thu, Nov 08, 2018 at 07:48:16AM +0100, Thomas Gleixner wrote:
Florian,
On Wed, 7 Nov 2018, Florian Fainelli wrote:
On 11/1/18 1:02 PM, Florian Fainelli wrote:
From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 78c9c4dfbf8c04883941445a195276bb4bb92c76 ]
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 [florian: Make patch apply to v4.9.135] Signed-off-by: Florian Fainelli f.fainelli@gmail.com
Thomas, can you review for correctness? Thanks!
Thomas, John, does that look like a reasonable backport for 4.9?
Looks correct.
Queued for 4.9, thanks all.
-- Thanks, Sasha
On 11/8/18 7:46 AM, Sasha Levin wrote:
On Thu, Nov 08, 2018 at 07:48:16AM +0100, Thomas Gleixner wrote:
Florian,
On Wed, 7 Nov 2018, Florian Fainelli wrote:
On 11/1/18 1:02 PM, Florian Fainelli wrote:
From: Thomas Gleixner tglx@linutronix.de
[ Upstream commit 78c9c4dfbf8c04883941445a195276bb4bb92c76 ]
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 [florian: Make patch apply to v4.9.135] Signed-off-by: Florian Fainelli f.fainelli@gmail.com
Thomas, can you review for correctness? Thanks!
Thomas, John, does that look like a reasonable backport for 4.9?
Looks correct.
Queued for 4.9, thanks all.
Thanks, the same patch should be applicable to earlier kernel versions as well with little to no hunks. At least I applied it as far back as 4.1 so far. I could provide a patch for earlier kernels that did not have the file renaming.
linux-stable-mirror@lists.linaro.org