timer: prevent overflow in apply_slack
PATCH 1/1 timer: prevent overflow in apply_slack
Prevent overflow in the computation of timer expiry time inside apply_slack().
kernel/timer.c +1 -1
@@ -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);
Signature · 2 lines
Jiri Bohac <[email protected]> SUSE Labs, SUSE CZ
Quoted 2 lines · Jiri Bohac
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
PATCH v2 1/1 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.
kernel/timer.c +1 -1
@@ -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);
Signature · 3 lines
-- Jiri Bohac <[email protected]> SUSE Labs, SUSE CZ
Thomas, does this make sense now, with the new description?
Quoted 21 lines · Jiri Bohac
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 =
0xffffe6f5and 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]>kernel/timer.c +0
-0
kernel/timer.c unsigned long apply_slack(struct timer_list *timer, unsigned long expires) · line 844
Quoted 8 lines
kernel/timer.c +1
-1@@ -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,
Signature · 2 lines
Jiri Bohac <[email protected]> SUSE Labs, SUSE CZ
Quoted 1 line · Jiri Bohac
Thomas, does this make sense now, with the new description?
Yep, except
Quoted 23 lines · Jiri Bohac
Quoted 22 lines
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 =
0xffffe6f5and 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]>kernel/timer.c +0
-0@@ -838,7 +838,7 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires)
Quoted 4 lines
kernel/timer.c unsigned long apply_slack(struct timer_list *timer, unsigned long expires) · line 841
Quoted 4 lines
kernel/timer.c +1
-1bit =find_last_bit (&mask,BITS_PER_LONG ); - mask = (1 << bit) -1 ; + mask = (1LL << bit) -1 ;
This should be 1UL, shouldn't it?
Quoted 10 lines
kernel/timer.c unsigned long apply_slack(struct timer_list *timer, unsigned long expires) · line 844
Quoted 2 lines
kernel/timer.c +0
-0expires_limit = expires_limit & ~(mask);Thanks,
Signature · 2 lines
Jiri Bohac <[email protected]> SUSE Labs, SUSE CZ
Quoted 3 lines · Thomas Gleixner
Quoted 1 line
Review
Quoted 1 line
(quoted diff) +1
-0+ 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.
kernel/timer.c +1 -1
@@ -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);
Signature · 2 lines
Jiri Bohac <[email protected]> SUSE Labs, SUSE CZ
tip-bot for Jiri Bohac
merge bot[tip:timers/urgent] timer: Prevent overflow in apply_slack
In reply to Jiri Bohactimer: Prevent overflow in apply_slack
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.
--- kernel/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/timer.c +1 -1
@@ -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);