All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] timer: prevent overflow in apply_slack
@ 2014-04-17 19:42 Jiri Bohac
  2014-04-17 21:46 ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: Jiri Bohac @ 2014-04-17 19:42 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel

Prevent overflow in the computation of timer expiry time inside
apply_slack().

Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Suggested-by: Deborah Townsend <dstownse@us.ibm.com>

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 <jbohac@suse.cz>
SUSE Labs, SUSE CZ


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] timer: prevent overflow in apply_slack
  2014-04-17 19:42 [PATCH] timer: prevent overflow in apply_slack Jiri Bohac
@ 2014-04-17 21:46 ` Thomas Gleixner
  2014-04-18 15:23   ` [PATCH v2] " Jiri Bohac
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2014-04-17 21:46 UTC (permalink / raw)
  To: Jiri Bohac; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] timer: prevent overflow in apply_slack
  2014-04-17 21:46 ` Thomas Gleixner
@ 2014-04-18 15:23   ` Jiri Bohac
  2014-04-29 11:13     ` Jiri Bohac
  2014-04-30 11:48     ` [tip:timers/urgent] timer: Prevent " tip-bot for Jiri Bohac
  0 siblings, 2 replies; 7+ messages in thread
From: Jiri Bohac @ 2014-04-18 15:23 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Jiri Bohac, linux-kernel

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 <jbohac@suse.cz>
Suggested-by: Deborah Townsend <dstownse@us.ibm.com>

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 <jbohac@suse.cz>
SUSE Labs, SUSE CZ


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] timer: prevent overflow in apply_slack
  2014-04-18 15:23   ` [PATCH v2] " Jiri Bohac
@ 2014-04-29 11:13     ` Jiri Bohac
  2014-04-29 17:22       ` Thomas Gleixner
  2014-04-30 11:48     ` [tip:timers/urgent] timer: Prevent " tip-bot for Jiri Bohac
  1 sibling, 1 reply; 7+ messages in thread
From: Jiri Bohac @ 2014-04-29 11:13 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel

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 <jbohac@suse.cz>
> Suggested-by: Deborah Townsend <dstownse@us.ibm.com>
> 
> 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 <jbohac@suse.cz>
SUSE Labs, SUSE CZ


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] timer: prevent overflow in apply_slack
  2014-04-29 11:13     ` Jiri Bohac
@ 2014-04-29 17:22       ` Thomas Gleixner
  2014-04-30  9:33         ` [PATCH v3] " Jiri Bohac
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2014-04-29 17:22 UTC (permalink / raw)
  To: Jiri Bohac; +Cc: linux-kernel

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 <jbohac@suse.cz>
> > Suggested-by: Deborah Townsend <dstownse@us.ibm.com>
> > 
> > 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 <jbohac@suse.cz>
> SUSE Labs, SUSE CZ
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] timer: prevent overflow in apply_slack
  2014-04-29 17:22       ` Thomas Gleixner
@ 2014-04-30  9:33         ` Jiri Bohac
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Bohac @ 2014-04-30  9:33 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: linux-kernel

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 <jbohac@suse.cz>
Suggested-by: Deborah Townsend <dstownse@us.ibm.com>

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 <jbohac@suse.cz>
SUSE Labs, SUSE CZ


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [tip:timers/urgent] timer: Prevent overflow in apply_slack
  2014-04-18 15:23   ` [PATCH v2] " Jiri Bohac
  2014-04-29 11:13     ` Jiri Bohac
@ 2014-04-30 11:48     ` tip-bot for Jiri Bohac
  1 sibling, 0 replies; 7+ messages in thread
From: tip-bot for Jiri Bohac @ 2014-04-30 11:48 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, dstownse, hpa, mingo, jbohac, tglx

Commit-ID:  98a01e779f3c66b0b11cd7e64d531c0e41c95762
Gitweb:     http://git.kernel.org/tip/98a01e779f3c66b0b11cd7e64d531c0e41c95762
Author:     Jiri Bohac <jbohac@suse.cz>
AuthorDate: Fri, 18 Apr 2014 17:23:11 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
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 <dstownse@us.ibm.com>
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20140418152310.GA13654@midget.suse.cz
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-04-30 11:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-17 19:42 [PATCH] timer: prevent overflow in apply_slack Jiri Bohac
2014-04-17 21:46 ` Thomas Gleixner
2014-04-18 15:23   ` [PATCH v2] " Jiri Bohac
2014-04-29 11:13     ` Jiri Bohac
2014-04-29 17:22       ` Thomas Gleixner
2014-04-30  9:33         ` [PATCH v3] " Jiri Bohac
2014-04-30 11:48     ` [tip:timers/urgent] timer: Prevent " tip-bot for Jiri Bohac

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.