[SRU][Trusty][Xenial][Bionic][PATCH 0/1] Fix for CVE-2018-12896

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][Xenial][Bionic][PATCH 0/1] Fix for CVE-2018-12896

Khaled Elmously
Backport of 78c9c4dfbf8c04883941445a195276bb4bb92c76 to fix CVE-2018-12896

Clean pick for Bionic, backported needed for Trusty and Xenial.

Thomas Gleixner (1):
  posix-timers: Sanitize overrun handling

 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(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][PATCH 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
From: Thomas Gleixner <[hidden email]>

CVE-2018-12896

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(cherry picked from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 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 672c4f32311e..437a539898ae 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -82,8 +82,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;
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 1f27887aa194..0f5f019c6645 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -84,7 +84,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;
  }
 }
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 708992708332..3192a986a483 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -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,9 +301,8 @@ 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_interval);
+ 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,9 +428,8 @@ 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_interval);
+ timr->it_overrun += hrtimer_forward(timer, now,
+    timr->it_interval);
  ret = HRTIMER_RESTART;
  ++timr->it_requeue_pending;
  timr->it_active = 1;
@@ -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();
@@ -789,7 +798,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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
From: Thomas Gleixner <[hidden email]>

CVE-2018-12896

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 include/linux/posix-timers.h |  4 ++--
 kernel/posix-cpu-timers.c    |  8 ++++----
 kernel/posix-timers.c        | 29 +++++++++++++++++++----------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c7f31aa272f7..0f62265a707c 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/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;
  }
 }
@@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
  timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
  ~REQUEUE_PENDING;
  timer->it_overrun_last = 0;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
 
  if (new_expires != 0 && !(val < new_expires)) {
  /*
@@ -1119,7 +1119,7 @@ out_unlock:
 
 out:
  timer->it_overrun_last = timer->it_overrun;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
  ++timer->it_requeue_pending;
 }
 
@@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
  memset(&timer, 0, sizeof timer);
  spin_lock_init(&timer.it_lock);
  timer.it_clock = which_clock;
- timer.it_overrun = -1;
+ timer.it_overrun = -1LL;
  error = posix_cpu_timer_create(&timer);
  timer.it_process = current;
  if (!error) {
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 26b910462a0c..2577e33bc964 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -346,6 +346,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;
@@ -353,12 +364,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);
 }
@@ -387,7 +397,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)
@@ -482,9 +492,8 @@ 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);
+ timr->it_overrun += hrtimer_forward(timer, now,
+    timr->it.real.interval);
  ret = HRTIMER_RESTART;
  ++timr->it_requeue_pending;
  }
@@ -624,7 +633,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))) {
@@ -816,7 +825,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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
From: Thomas Gleixner <[hidden email]>

CVE-2018-12896

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 include/linux/posix-timers.h   |  4 ++--
 kernel/time/posix-cpu-timers.c |  8 ++++----
 kernel/time/posix-timers.c     | 28 ++++++++++++++++++----------
 3 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd191ac..3e28a1a8d823 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 80016b329d94..9b6015527482 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;
  }
 }
@@ -744,7 +744,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
  timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
  ~REQUEUE_PENDING;
  timer->it_overrun_last = 0;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
 
  if (new_expires != 0 && !(val < new_expires)) {
  /*
@@ -1100,7 +1100,7 @@ void posix_cpu_timer_schedule(struct k_itimer *timer)
  posix_cpu_timer_kick_nohz();
 out:
  timer->it_overrun_last = timer->it_overrun;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
  ++timer->it_requeue_pending;
 }
 
@@ -1305,7 +1305,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
  memset(&timer, 0, sizeof timer);
  spin_lock_init(&timer.it_lock);
  timer.it_clock = which_clock;
- timer.it_overrun = -1;
+ timer.it_overrun = -1LL;
  error = posix_cpu_timer_create(&timer);
  timer.it_process = current;
  if (!error) {
diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index fc7c37ad90a0..d34a7f0b92fe 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -354,6 +354,16 @@ 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)
 {
@@ -362,12 +372,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 +405,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 +500,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 +641,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 +770,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 +832,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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Trusty][PATCH 1/1] posix-timers: Sanitize overrun handling

Tyler Hicks-2
In reply to this post by Khaled Elmously
On 2018-11-27 00:23:17, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h |  4 ++--
>  kernel/posix-cpu-timers.c    |  8 ++++----
>  kernel/posix-timers.c        | 29 +++++++++++++++++++----------
>  3 files changed, 25 insertions(+), 16 deletions(-)
This seems like some potentially hair backports to get right. I'm still
going through the Xenial and Trusty patches but I noticed that you
dropped the hunk that removes the (unsigned int) cast in
common_timer_get() for the Trusty backport. I don't think that was
intentional but let me know if it was.

Tyler

>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index c7f31aa272f7..0f62265a707c 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/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;
>   }
>  }
> @@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
>   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
>   ~REQUEUE_PENDING;
>   timer->it_overrun_last = 0;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>  
>   if (new_expires != 0 && !(val < new_expires)) {
>   /*
> @@ -1119,7 +1119,7 @@ out_unlock:
>  
>  out:
>   timer->it_overrun_last = timer->it_overrun;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>   ++timer->it_requeue_pending;
>  }
>  
> @@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>   memset(&timer, 0, sizeof timer);
>   spin_lock_init(&timer.it_lock);
>   timer.it_clock = which_clock;
> - timer.it_overrun = -1;
> + timer.it_overrun = -1LL;
>   error = posix_cpu_timer_create(&timer);
>   timer.it_process = current;
>   if (!error) {
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 26b910462a0c..2577e33bc964 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -346,6 +346,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;
> @@ -353,12 +364,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);
>  }
> @@ -387,7 +397,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)
> @@ -482,9 +492,8 @@ 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);
> + timr->it_overrun += hrtimer_forward(timer, now,
> +    timr->it.real.interval);
>   ret = HRTIMER_RESTART;
>   ++timr->it_requeue_pending;
>   }
> @@ -624,7 +633,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))) {
> @@ -816,7 +825,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;
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][PATCH 1/1] posix-timers: Sanitize overrun handling

Tyler Hicks-2
In reply to this post by Khaled Elmously
On 2018-11-27 00:23:16, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (cherry picked from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> Signed-off-by: Khalid Elmously <[hidden email]>
Acked-by: Tyler Hicks <[hidden email]>

This ack is only for the Bionic patch. Thanks!

Tyler

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK/CMNT: [SRU][Xenial][PATCH 1/1] posix-timers: Sanitize overrun handling

Tyler Hicks-2
In reply to this post by Khaled Elmously
On 2018-11-27 00:23:18, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> Signed-off-by: Khalid Elmously <[hidden email]>
Acked-by: Tyler Hicks <[hidden email]>

See inline comments. Thanks!

> ---
>  include/linux/posix-timers.h   |  4 ++--
>  kernel/time/posix-cpu-timers.c |  8 ++++----
>  kernel/time/posix-timers.c     | 28 ++++++++++++++++++----------
>  3 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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 80016b329d94..9b6015527482 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;
>   }
>  }
> @@ -744,7 +744,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
>   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
>   ~REQUEUE_PENDING;
>   timer->it_overrun_last = 0;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
You added an "LL" suffix here that wasn't in the original patch. I'm not
sure if it was intentional but I also don't think it hurts anything.

>  
>   if (new_expires != 0 && !(val < new_expires)) {
>   /*
> @@ -1100,7 +1100,7 @@ void posix_cpu_timer_schedule(struct k_itimer *timer)
>   posix_cpu_timer_kick_nohz();
>  out:
>   timer->it_overrun_last = timer->it_overrun;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;

Same here although posix_cpu_timer_schedule() and this assignment isn't
present in the current upstream kernel.

>   ++timer->it_requeue_pending;
>  }
>  
> @@ -1305,7 +1305,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>   memset(&timer, 0, sizeof timer);
>   spin_lock_init(&timer.it_lock);
>   timer.it_clock = which_clock;
> - timer.it_overrun = -1;
> + timer.it_overrun = -1LL;

Same here.

Tyler

>   error = posix_cpu_timer_create(&timer);
>   timer.it_process = current;
>   if (!error) {
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index fc7c37ad90a0..d34a7f0b92fe 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -354,6 +354,16 @@ 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)
>  {
> @@ -362,12 +372,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 +405,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 +500,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 +641,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 +770,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 +832,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;
> --
> 2.17.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [SRU][Trusty][PATCH 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Tyler Hicks-2
On 2018-11-27 12:10:51 , Tyler Hicks wrote:

> On 2018-11-27 00:23:17, Khalid Elmously wrote:
> > From: Thomas Gleixner <[hidden email]>
> >
> > CVE-2018-12896
> >
> > 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 <[hidden email]>
> > Signed-off-by: Thomas Gleixner <[hidden email]>
> > Acked-by: John Stultz <[hidden email]>
> > Cc: Peter Zijlstra <[hidden email]>
> > Cc: Michael Kerrisk <[hidden email]>
> > Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> > (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> > Signed-off-by: Khalid Elmously <[hidden email]>
> > ---
> >  include/linux/posix-timers.h |  4 ++--
> >  kernel/posix-cpu-timers.c    |  8 ++++----
> >  kernel/posix-timers.c        | 29 +++++++++++++++++++----------
> >  3 files changed, 25 insertions(+), 16 deletions(-)
>
> This seems like some potentially hair backports to get right. I'm still
> going through the Xenial and Trusty patches but I noticed that you
> dropped the hunk that removes the (unsigned int) cast in
> common_timer_get() for the Trusty backport. I don't think that was
> intentional but let me know if it was.

Yes, that was a mistake on my part. Thank you for catching it! (I don't think the cast is harmful anyway, but better to be consistent)

Will send a v2


>
> Tyler
>
> >
> > diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> > index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> > index c7f31aa272f7..0f62265a707c 100644
> > --- a/kernel/posix-cpu-timers.c
> > +++ b/kernel/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;
> >   }
> >  }
> > @@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
> >   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
> >   ~REQUEUE_PENDING;
> >   timer->it_overrun_last = 0;
> > - timer->it_overrun = -1;
> > + timer->it_overrun = -1LL;
> >  
> >   if (new_expires != 0 && !(val < new_expires)) {
> >   /*
> > @@ -1119,7 +1119,7 @@ out_unlock:
> >  
> >  out:
> >   timer->it_overrun_last = timer->it_overrun;
> > - timer->it_overrun = -1;
> > + timer->it_overrun = -1LL;
> >   ++timer->it_requeue_pending;
> >  }
> >  
> > @@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
> >   memset(&timer, 0, sizeof timer);
> >   spin_lock_init(&timer.it_lock);
> >   timer.it_clock = which_clock;
> > - timer.it_overrun = -1;
> > + timer.it_overrun = -1LL;
> >   error = posix_cpu_timer_create(&timer);
> >   timer.it_process = current;
> >   if (!error) {
> > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> > index 26b910462a0c..2577e33bc964 100644
> > --- a/kernel/posix-timers.c
> > +++ b/kernel/posix-timers.c
> > @@ -346,6 +346,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;
> > @@ -353,12 +364,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);
> >  }
> > @@ -387,7 +397,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)
> > @@ -482,9 +492,8 @@ 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);
> > + timr->it_overrun += hrtimer_forward(timer, now,
> > +    timr->it.real.interval);
> >   ret = HRTIMER_RESTART;
> >   ++timr->it_requeue_pending;
> >   }
> > @@ -624,7 +633,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))) {
> > @@ -816,7 +825,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;
> > --
> > 2.17.1
> >
> >
> > --
> > kernel-team mailing list
> > [hidden email]
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
From: Thomas Gleixner <[hidden email]>

CVE-2018-12896

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
Signed-off-by: Khalid Elmously <[hidden email]>
---
 include/linux/posix-timers.h |  4 ++--
 kernel/posix-cpu-timers.c    |  8 ++++----
 kernel/posix-timers.c        | 31 ++++++++++++++++++++-----------
 3 files changed, 26 insertions(+), 17 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c7f31aa272f7..0f62265a707c 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/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;
  }
 }
@@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
  timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
  ~REQUEUE_PENDING;
  timer->it_overrun_last = 0;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
 
  if (new_expires != 0 && !(val < new_expires)) {
  /*
@@ -1119,7 +1119,7 @@ out_unlock:
 
 out:
  timer->it_overrun_last = timer->it_overrun;
- timer->it_overrun = -1;
+ timer->it_overrun = -1LL;
  ++timer->it_requeue_pending;
 }
 
@@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
  memset(&timer, 0, sizeof timer);
  spin_lock_init(&timer.it_lock);
  timer.it_clock = which_clock;
- timer.it_overrun = -1;
+ timer.it_overrun = -1LL;
  error = posix_cpu_timer_create(&timer);
  timer.it_process = current;
  if (!error) {
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 26b910462a0c..8ef1ab65fe84 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -346,6 +346,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;
@@ -353,12 +364,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);
 }
@@ -387,7 +397,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)
@@ -482,9 +492,8 @@ 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);
+ timr->it_overrun += hrtimer_forward(timer, now,
+    timr->it.real.interval);
  ret = HRTIMER_RESTART;
  ++timr->it_requeue_pending;
  }
@@ -624,7 +633,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))) {
@@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
- timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
+ timr->it_overrun += hrtimer_forward(timer, now, iv);
 
  remaining = ktime_sub(hrtimer_get_expires(timer), now);
  /* Return 0 only, when the timer is expired and not pending */
@@ -816,7 +825,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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][PATCH 1/1] posix-timers: Sanitize overrun handling

Kleber Souza
In reply to this post by Khaled Elmously
On 11/27/18 6:23 AM, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (cherry picked from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> Signed-off-by: Khalid Elmously <[hidden email]>


Acked-by: Kleber Sacilotto de Souza <[hidden email]>


> ---
>  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 672c4f32311e..437a539898ae 100644
> --- a/include/linux/posix-timers.h
> +++ b/include/linux/posix-timers.h
> @@ -82,8 +82,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;
> diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
> index 1f27887aa194..0f5f019c6645 100644
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -84,7 +84,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;
>   }
>  }
> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index 708992708332..3192a986a483 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -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,9 +301,8 @@ 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_interval);
> + 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,9 +428,8 @@ 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_interval);
> + timr->it_overrun += hrtimer_forward(timer, now,
> +    timr->it_interval);
>   ret = HRTIMER_RESTART;
>   ++timr->it_requeue_pending;
>   timr->it_active = 1;
> @@ -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();
> @@ -789,7 +798,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;



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NAK: [SRU][Trusty][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Kleber Souza
In reply to this post by Khaled Elmously
On 11/28/18 6:19 AM, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
When some changes more than simple context adjustments are needed,
please add a few words explaining what was needed for the backport. In
this case it would be relevant to mention that the path of some files
were different (e.g. kernel/time/posix-cpu-timers.c ->
kernel/posix-cpu-timers.c), etc. That helps the people reviewing and
looking at the code later to easily spot the difference from the
original patch.

> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h |  4 ++--
>  kernel/posix-cpu-timers.c    |  8 ++++----
>  kernel/posix-timers.c        | 31 ++++++++++++++++++++-----------
>  3 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index c7f31aa272f7..0f62265a707c 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/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;
>   }
>  }
> @@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
>   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
>   ~REQUEUE_PENDING;
>   timer->it_overrun_last = 0;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>  
>   if (new_expires != 0 && !(val < new_expires)) {
>   /*
> @@ -1119,7 +1119,7 @@ out_unlock:
>  
>  out:
>   timer->it_overrun_last = timer->it_overrun;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>   ++timer->it_requeue_pending;
>  }
>  
> @@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>   memset(&timer, 0, sizeof timer);
>   spin_lock_init(&timer.it_lock);
>   timer.it_clock = which_clock;
> - timer.it_overrun = -1;
> + timer.it_overrun = -1LL;


I know the above changes and some below from -1 to -1LL on the
assignments makes sense to have a consistency with the changes made on
the other files, but unless there's a very strong reason to divert from
mainline we tend to not make changes on the code that wasn't on the
original patch and would make the code different from mainline. The
reason for this is that cherry-picking or backporting patches in the
future can demand some extra work because of the diversion. If you
really think this should be changed you can send a patch upstream
suggesting it :-).

So that's why I'm NAK'ing it and will do for the Xenial one as well.

Could you please resend with changes to make it as close as possible to
the original patch?


Thanks,

Kleber

>   error = posix_cpu_timer_create(&timer);
>   timer.it_process = current;
>   if (!error) {
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 26b910462a0c..8ef1ab65fe84 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -346,6 +346,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;
> @@ -353,12 +364,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);
>  }
> @@ -387,7 +397,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)
> @@ -482,9 +492,8 @@ 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);
> + timr->it_overrun += hrtimer_forward(timer, now,
> +    timr->it.real.interval);
>   ret = HRTIMER_RESTART;
>   ++timr->it_requeue_pending;
>   }
> @@ -624,7 +633,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))) {
> @@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
> - timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
> + timr->it_overrun += hrtimer_forward(timer, now, iv);
>  
>   remaining = ktime_sub(hrtimer_get_expires(timer), now);
>   /* Return 0 only, when the timer is expired and not pending */
> @@ -816,7 +825,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;




--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

NAK: [SRU][Xenial][PATCH 1/1] posix-timers: Sanitize overrun handling

Kleber Souza
In reply to this post by Khaled Elmously
On 11/27/18 6:23 AM, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> CVE-2018-12896
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h   |  4 ++--
>  kernel/time/posix-cpu-timers.c |  8 ++++----
>  kernel/time/posix-timers.c     | 28 ++++++++++++++++++----------
>  3 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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 80016b329d94..9b6015527482 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;
>   }
>  }
> @@ -744,7 +744,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
>   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
>   ~REQUEUE_PENDING;
>   timer->it_overrun_last = 0;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>  
>   if (new_expires != 0 && !(val < new_expires)) {
>   /*
> @@ -1100,7 +1100,7 @@ void posix_cpu_timer_schedule(struct k_itimer *timer)
>   posix_cpu_timer_kick_nohz();
>  out:
>   timer->it_overrun_last = timer->it_overrun;
> - timer->it_overrun = -1;
> + timer->it_overrun = -1LL;
>   ++timer->it_requeue_pending;
>  }
>  
> @@ -1305,7 +1305,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
>   memset(&timer, 0, sizeof timer);
>   spin_lock_init(&timer.it_lock);
>   timer.it_clock = which_clock;
> - timer.it_overrun = -1;
> + timer.it_overrun = -1LL;
>   error = posix_cpu_timer_create(&timer);
>   timer.it_process = current;
>   if (!error) {

Same comments here as from the patch for Trusty. These changes would be
nice, but I'm not sure diverting from mainline would be worth.


Thanks,

Kleber


> diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
> index fc7c37ad90a0..d34a7f0b92fe 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -354,6 +354,16 @@ 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)
>  {
> @@ -362,12 +372,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 +405,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 +500,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 +641,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 +770,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 +832,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;



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: NAK: [SRU][Trusty][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Kleber Souza
On 2018-11-28 17:38:17 , Kleber Souza wrote:

> On 11/28/18 6:19 AM, Khalid Elmously wrote:
> > From: Thomas Gleixner <[hidden email]>
> >
> > CVE-2018-12896
> >
> > 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 <[hidden email]>
> > Signed-off-by: Thomas Gleixner <[hidden email]>
> > Acked-by: John Stultz <[hidden email]>
> > Cc: Peter Zijlstra <[hidden email]>
> > Cc: Michael Kerrisk <[hidden email]>
> > Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> > (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> When some changes more than simple context adjustments are needed,
> please add a few words explaining what was needed for the backport. In
> this case it would be relevant to mention that the path of some files
> were different (e.g. kernel/time/posix-cpu-timers.c ->
> kernel/posix-cpu-timers.c), etc. That helps the people reviewing and
> looking at the code later to easily spot the difference from the
> original patch.

Ack

> > Signed-off-by: Khalid Elmously <[hidden email]>
> > ---
> >  include/linux/posix-timers.h |  4 ++--
> >  kernel/posix-cpu-timers.c    |  8 ++++----
> >  kernel/posix-timers.c        | 31 ++++++++++++++++++++-----------
> >  3 files changed, 26 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> > index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> > index c7f31aa272f7..0f62265a707c 100644
> > --- a/kernel/posix-cpu-timers.c
> > +++ b/kernel/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;
> >   }
> >  }
> > @@ -761,7 +761,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int flags,
> >   timer->it_requeue_pending = (timer->it_requeue_pending + 2) &
> >   ~REQUEUE_PENDING;
> >   timer->it_overrun_last = 0;
> > - timer->it_overrun = -1;
> > + timer->it_overrun = -1LL;
> >  
> >   if (new_expires != 0 && !(val < new_expires)) {
> >   /*
> > @@ -1119,7 +1119,7 @@ out_unlock:
> >  
> >  out:
> >   timer->it_overrun_last = timer->it_overrun;
> > - timer->it_overrun = -1;
> > + timer->it_overrun = -1LL;
> >   ++timer->it_requeue_pending;
> >  }
> >  
> > @@ -1327,7 +1327,7 @@ static int do_cpu_nanosleep(const clockid_t which_clock, int flags,
> >   memset(&timer, 0, sizeof timer);
> >   spin_lock_init(&timer.it_lock);
> >   timer.it_clock = which_clock;
> > - timer.it_overrun = -1;
> > + timer.it_overrun = -1LL;
>
>
> I know the above changes and some below from -1 to -1LL on the
> assignments makes sense to have a consistency with the changes made on
> the other files, but unless there's a very strong reason to divert from
> mainline we tend to not make changes on the code that wasn't on the
> original patch and would make the code different from mainline. The
> reason for this is that cherry-picking or backporting patches in the
> future can demand some extra work because of the diversion. If you
> really think this should be changed you can send a patch upstream
> suggesting it :-).
>
> So that's why I'm NAK'ing it and will do for the Xenial one as well.
>
> Could you please resend with changes to make it as close as possible to
> the original patch?
>

Thanks for the explanation and for the review.
I will send an update

Khalid

>
> Thanks,
>
> Kleber
>
> >   error = posix_cpu_timer_create(&timer);
> >   timer.it_process = current;
> >   if (!error) {
> > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> > index 26b910462a0c..8ef1ab65fe84 100644
> > --- a/kernel/posix-timers.c
> > +++ b/kernel/posix-timers.c
> > @@ -346,6 +346,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;
> > @@ -353,12 +364,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);
> >  }
> > @@ -387,7 +397,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)
> > @@ -482,9 +492,8 @@ 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);
> > + timr->it_overrun += hrtimer_forward(timer, now,
> > +    timr->it.real.interval);
> >   ret = HRTIMER_RESTART;
> >   ++timr->it_requeue_pending;
> >   }
> > @@ -624,7 +633,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))) {
> > @@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
> > - timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
> > + timr->it_overrun += hrtimer_forward(timer, now, iv);
> >  
> >   remaining = ktime_sub(hrtimer_get_expires(timer), now);
> >   /* Return 0 only, when the timer is expired and not pending */
> > @@ -816,7 +825,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;
>
>
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Xenial][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
From: Thomas Gleixner <[hidden email]>

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
[kmously: Mostly context adjustments or adjusting for slightly refactored
code. A cosmetic change from the original (changing '-1' to
'-1LL') was dropped because it's not applicable ]
Signed-off-by: Khalid Elmously <[hidden email]>
---
 include/linux/posix-timers.h   |  4 ++--
 kernel/time/posix-cpu-timers.c |  2 +-
 kernel/time/posix-timers.c     | 23 ++++++++++++++++-------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd191ac..3e28a1a8d823 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 80016b329d94..8fc68e60c795 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..cb77eeab669f 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -354,6 +354,16 @@ 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)
 {
@@ -362,7 +372,7 @@ 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,
+ timr->it_overrun += hrtimer_forward(timer,
  timer->base->get_time(),
  timr->it.real.interval);
 
@@ -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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Trusty][PATCH v3 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
From: Thomas Gleixner <[hidden email]>

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 <[hidden email]>
Signed-off-by: Thomas Gleixner <[hidden email]>
Acked-by: John Stultz <[hidden email]>
Cc: Peter Zijlstra <[hidden email]>
Cc: Michael Kerrisk <[hidden email]>
Link: https://lkml.kernel.org/r/20180626132705.018623573@...
(backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
[kmously: Mostly context adjustments or adjusting for slightly refactored
code. A cosmetic change from the original (changing '-1' to
'-1LL') was dropped because it's not applicable. Also path changes were
needed from kernel/time -> kernel ]
Signed-off-by: Khalid Elmously <[hidden email]>
---
 include/linux/posix-timers.h |  4 ++--
 kernel/posix-cpu-timers.c    |  2 +-
 kernel/posix-timers.c        | 23 ++++++++++++++++-------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
index c7f31aa272f7..bd2f746e837c 100644
--- a/kernel/posix-cpu-timers.c
+++ b/kernel/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/posix-timers.c b/kernel/posix-timers.c
index 26b910462a0c..832a142ea556 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -345,6 +345,16 @@ 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)
 {
@@ -353,7 +363,7 @@ 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,
+ timr->it_overrun += hrtimer_forward(timer,
  timer->base->get_time(),
  timr->it.real.interval);
 
@@ -387,7 +397,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)
@@ -482,8 +492,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;
@@ -624,7 +633,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))) {
@@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
- timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
+ timr->it_overrun += hrtimer_forward(timer, now, iv);
 
  remaining = ktime_sub(hrtimer_get_expires(timer), now);
  /* Return 0 only, when the timer is expired and not pending */
@@ -816,7 +825,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;
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [SRU][Xenial][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Kleber Souza
In reply to this post by Khaled Elmously
On 11/29/18 7:42 AM, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> [kmously: Mostly context adjustments or adjusting for slightly refactored
> code. A cosmetic change from the original (changing '-1' to
> '-1LL') was dropped because it's not applicable ]
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h   |  4 ++--
>  kernel/time/posix-cpu-timers.c |  2 +-
>  kernel/time/posix-timers.c     | 23 ++++++++++++++++-------
>  3 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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 80016b329d94..8fc68e60c795 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..cb77eeab669f 100644
> --- a/kernel/time/posix-timers.c
> +++ b/kernel/time/posix-timers.c
> @@ -354,6 +354,16 @@ 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)
>  {
> @@ -362,7 +372,7 @@ 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,
> + timr->it_overrun += hrtimer_forward(timer,
>   timer->base->get_time(),
>   timr->it.real.interval);
It would be nice to reformat the indentation of the parameters as done
by the original patch. This also helps with future cherry-picks/backports.

>  
> @@ -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);
Same here.

>   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;

Apart from the indentation issues, which can be fixed when applying the
patch, the backport looks good.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


Thanks!



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [SRU][Trusty][PATCH v3 1/1] posix-timers: Sanitize overrun handling

Kleber Souza
In reply to this post by Khaled Elmously
On 11/29/18 7:43 AM, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> [kmously: Mostly context adjustments or adjusting for slightly refactored
> code. A cosmetic change from the original (changing '-1' to
> '-1LL') was dropped because it's not applicable. Also path changes were
> needed from kernel/time -> kernel ]
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h |  4 ++--
>  kernel/posix-cpu-timers.c    |  2 +-
>  kernel/posix-timers.c        | 23 ++++++++++++++++-------
>  3 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index c7f31aa272f7..bd2f746e837c 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/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/posix-timers.c b/kernel/posix-timers.c
> index 26b910462a0c..832a142ea556 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -345,6 +345,16 @@ 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)
>  {
> @@ -353,7 +363,7 @@ 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,
> + timr->it_overrun += hrtimer_forward(timer,
>   timer->base->get_time(),
>   timr->it.real.interval);

Same comment as the patch for Xenial, it would be nice to fix the
indentation of the parameters.

>  
> @@ -387,7 +397,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)
> @@ -482,8 +492,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);

Same here about the indentation.

>   ret = HRTIMER_RESTART;
>   ++timr->it_requeue_pending;
> @@ -624,7 +633,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))) {
> @@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
> - timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
> + timr->it_overrun += hrtimer_forward(timer, now, iv);
>  
>   remaining = ktime_sub(hrtimer_get_expires(timer), now);
>   /* Return 0 only, when the timer is expired and not pending */
> @@ -816,7 +825,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;

Also looks good, apart from the indentation issues that can be fixed
when applying it.

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


Thanks,

Kleber



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Xenial][PATCH v2 1/1] posix-timers: Sanitize overrun handling

Tyler Hicks-2
In reply to this post by Khaled Elmously
On 2018-11-29 01:42:41, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> [kmously: Mostly context adjustments or adjusting for slightly refactored
> code. A cosmetic change from the original (changing '-1' to
> '-1LL') was dropped because it's not applicable ]
> Signed-off-by: Khalid Elmously <[hidden email]>
Acked-by: Tyler Hicks <[hidden email]>

Thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Trusty][PATCH v3 1/1] posix-timers: Sanitize overrun handling

Tyler Hicks-2
In reply to this post by Khaled Elmously
On 2018-11-29 01:43:14, Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> [kmously: Mostly context adjustments or adjusting for slightly refactored
> code. A cosmetic change from the original (changing '-1' to
> '-1LL') was dropped because it's not applicable. Also path changes were
> needed from kernel/time -> kernel ]
> Signed-off-by: Khalid Elmously <[hidden email]>
Acked-by: Tyler Hicks <[hidden email]>

Tyler

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED/cmnt: [SRU][Trusty][PATCH v3 1/1] posix-timers: Sanitize overrun handling

Khaled Elmously
In reply to this post by Khaled Elmously
Applied, with the indentation fix that Kleber pointed out

On 2018-11-29 01:43:14 , Khalid Elmously wrote:

> From: Thomas Gleixner <[hidden email]>
>
> 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 <[hidden email]>
> Signed-off-by: Thomas Gleixner <[hidden email]>
> Acked-by: John Stultz <[hidden email]>
> Cc: Peter Zijlstra <[hidden email]>
> Cc: Michael Kerrisk <[hidden email]>
> Link: https://lkml.kernel.org/r/20180626132705.018623573@...
> (backported from commit 78c9c4dfbf8c04883941445a195276bb4bb92c76)
> [kmously: Mostly context adjustments or adjusting for slightly refactored
> code. A cosmetic change from the original (changing '-1' to
> '-1LL') was dropped because it's not applicable. Also path changes were
> needed from kernel/time -> kernel ]
> Signed-off-by: Khalid Elmously <[hidden email]>
> ---
>  include/linux/posix-timers.h |  4 ++--
>  kernel/posix-cpu-timers.c    |  2 +-
>  kernel/posix-timers.c        | 23 ++++++++++++++++-------
>  3 files changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 907f3fd191ac..3e28a1a8d823 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/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index c7f31aa272f7..bd2f746e837c 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/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/posix-timers.c b/kernel/posix-timers.c
> index 26b910462a0c..832a142ea556 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -345,6 +345,16 @@ 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)
>  {
> @@ -353,7 +363,7 @@ 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,
> + timr->it_overrun += hrtimer_forward(timer,
>   timer->base->get_time(),
>   timr->it.real.interval);
>  
> @@ -387,7 +397,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)
> @@ -482,8 +492,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;
> @@ -624,7 +633,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))) {
> @@ -754,7 +763,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_THREAD_ID) == SIGEV_NONE))
> - timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
> + timr->it_overrun += hrtimer_forward(timer, now, iv);
>  
>   remaining = ktime_sub(hrtimer_get_expires(timer), now);
>   /* Return 0 only, when the timer is expired and not pending */
> @@ -816,7 +825,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;
> --
> 2.17.1
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
12