Prevent overflow in the computation of timer expiry time inside
apply_slack().
Signed-off-by: Jiri Bohac <[email protected]>
Suggested-by: Deborah Townsend <[email protected]>
diff --git a/kernel/timer.c b/kernel/timer.c
index 87bd529..4c36c91 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
bit = find_last_bit(&mask, BITS_PER_LONG);
- mask = (1 << bit) - 1;
+ mask = (1LL << bit) - 1;
expires_limit = expires_limit & ~(mask);
--
Jiri Bohac <[email protected]>
SUSE Labs, SUSE CZ
On Thu, 17 Apr 2014, Jiri Bohac wrote:
> Prevent overflow in the computation of timer expiry time inside
> apply_slack().
What's the impact of this overflow?
We don't want changelogs which describe what they do, we want them to
describe WHY and WHAT kind of problem the patch fixes.
Thanks,
tglx
On architectures with sizeof(int) < sizeof (long), the
computation of mask inside apply_slack() can be undefined if the
computed bit is > 32.
E.g. with: expires = 0xffffe6f5 and slack = 25, we get:
expires_limit = 0x20000000e
bit = 33
mask = (1 << 33) - 1 /* undefined */
On x86, mask becomes 1 and and the slack is not applied properly.
On s390, mask is -1, expires is set to 0 and the timer fires immediately.
Signed-off-by: Jiri Bohac <[email protected]>
Suggested-by: Deborah Townsend <[email protected]>
diff --git a/kernel/timer.c b/kernel/timer.c
index 87bd529..4c36c91 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
bit = find_last_bit(&mask, BITS_PER_LONG);
- mask = (1 << bit) - 1;
+ mask = (1LL << bit) - 1;
expires_limit = expires_limit & ~(mask);
--
--
Jiri Bohac <[email protected]>
SUSE Labs, SUSE CZ
Thomas, does this make sense now, with the new description?
On Fri, Apr 18, 2014 at 05:23:11PM +0200, Jiri Bohac wrote:
> On architectures with sizeof(int) < sizeof (long), the
> computation of mask inside apply_slack() can be undefined if the
> computed bit is > 32.
>
> E.g. with: expires = 0xffffe6f5 and slack = 25, we get:
>
> expires_limit = 0x20000000e
> bit = 33
> mask = (1 << 33) - 1 /* undefined */
>
> On x86, mask becomes 1 and and the slack is not applied properly.
> On s390, mask is -1, expires is set to 0 and the timer fires immediately.
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Suggested-by: Deborah Townsend <[email protected]>
>
> diff --git a/kernel/timer.c b/kernel/timer.c
> index 87bd529..4c36c91 100644
> --- a/kernel/timer.c
> +++ b/kernel/timer.c
> @@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
>
> bit = find_last_bit(&mask, BITS_PER_LONG);
>
> - mask = (1 << bit) - 1;
> + mask = (1LL << bit) - 1;
>
> expires_limit = expires_limit & ~(mask);
>
Thanks,
--
Jiri Bohac <[email protected]>
SUSE Labs, SUSE CZ
On Tue, 29 Apr 2014, Jiri Bohac wrote:
> Thomas, does this make sense now, with the new description?
Yep, except
> On Fri, Apr 18, 2014 at 05:23:11PM +0200, Jiri Bohac wrote:
> > On architectures with sizeof(int) < sizeof (long), the
> > computation of mask inside apply_slack() can be undefined if the
> > computed bit is > 32.
> >
> > E.g. with: expires = 0xffffe6f5 and slack = 25, we get:
> >
> > expires_limit = 0x20000000e
> > bit = 33
> > mask = (1 << 33) - 1 /* undefined */
> >
> > On x86, mask becomes 1 and and the slack is not applied properly.
> > On s390, mask is -1, expires is set to 0 and the timer fires immediately.
> >
> > Signed-off-by: Jiri Bohac <[email protected]>
> > Suggested-by: Deborah Townsend <[email protected]>
> >
> > diff --git a/kernel/timer.c b/kernel/timer.c
> > index 87bd529..4c36c91 100644
> > --- a/kernel/timer.c
> > +++ b/kernel/timer.c
> > @@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
> >
> > bit = find_last_bit(&mask, BITS_PER_LONG);
> >
> > - mask = (1 << bit) - 1;
> > + mask = (1LL << bit) - 1;
This should be 1UL, shouldn't it?
> > expires_limit = expires_limit & ~(mask);
> >
>
> Thanks,
>
> --
> Jiri Bohac <[email protected]>
> SUSE Labs, SUSE CZ
>
>
On Tue, Apr 29, 2014 at 07:22:25PM +0200, Thomas Gleixner wrote:
> > > + mask = (1LL << bit) - 1;
>
> This should be 1UL, shouldn't it?
yes, good catch, thanks!
On architectures with sizeof(int) < sizeof (long), the
computation of mask inside apply_slack() can be undefined if the
computed bit is > 32.
E.g. with: expires = 0xffffe6f5 and slack = 25, we get:
expires_limit = 0x20000000e
bit = 33
mask = (1 << 33) - 1 /* undefined */
On x86, mask becomes 1 and and the slack is not applied properly.
On s390, mask is -1, expires is set to 0 and the timer fires immediately.
Signed-off-by: Jiri Bohac <[email protected]>
Suggested-by: Deborah Townsend <[email protected]>
diff --git a/kernel/timer.c b/kernel/timer.c
index 87bd529..4c36c91 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
bit = find_last_bit(&mask, BITS_PER_LONG);
- mask = (1 << bit) - 1;
+ mask = (1UL << bit) - 1;
expires_limit = expires_limit & ~(mask);
--
Jiri Bohac <[email protected]>
SUSE Labs, SUSE CZ
Commit-ID: 98a01e779f3c66b0b11cd7e64d531c0e41c95762
Gitweb: http://git.kernel.org/tip/98a01e779f3c66b0b11cd7e64d531c0e41c95762
Author: Jiri Bohac <[email protected]>
AuthorDate: Fri, 18 Apr 2014 17:23:11 +0200
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 30 Apr 2014 13:46:17 +0200
timer: Prevent overflow in apply_slack
On architectures with sizeof(int) < sizeof (long), the
computation of mask inside apply_slack() can be undefined if the
computed bit is > 32.
E.g. with: expires = 0xffffe6f5 and slack = 25, we get:
expires_limit = 0x20000000e
bit = 33
mask = (1 << 33) - 1 /* undefined */
On x86, mask becomes 1 and and the slack is not applied properly.
On s390, mask is -1, expires is set to 0 and the timer fires immediately.
Use 1UL << bit to solve that issue.
Suggested-by: Deborah Townsend <[email protected]>
Signed-off-by: Jiri Bohac <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/timer.c b/kernel/timer.c
index 87bd529..3bb01a3 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
bit = find_last_bit(&mask, BITS_PER_LONG);
- mask = (1 << bit) - 1;
+ mask = (1UL << bit) - 1;
expires_limit = expires_limit & ~(mask);