* x86_64 system lockup from userspace using setitimer()
@ 2007-03-13 18:55 Johannes Bauer
2007-03-13 19:19 ` Andreas Schwab
2007-03-13 20:02 ` Chuck Ebbert
0 siblings, 2 replies; 16+ messages in thread
From: Johannes Bauer @ 2007-03-13 18:55 UTC (permalink / raw)
To: linux-kernel
Dear Community,
I think I've encountered a bug with the Linux kernel which results in a
complete system lockup and which can be started without root priviliges.
It's reproducible with 2.6.20.1 and 2.6.20.2 and only x64_64 seems affected.
Here's the code which triggers the bug (originally found by me using an
only partly initialized "struct itimerval" structure - hence the strange
values in it_interval):
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
#include <stdio.h>
#include <sys/time.h>
#include <unistd.h>
int main(int argc, char **argv) {
struct itimerval tim = {
.it_interval = {
.tv_sec = 140735669863712,
.tv_usec = 4199521
},
.it_value = {
.tv_sec = 0,
.tv_usec = 100000
}
};
setitimer(ITIMER_REAL, &tim, NULL);
while (1) sleep(1);
return 0;
}
-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----
Compiled with gcc 4.1.1 with "gcc -O2 -Wall -o crash crash.c".
The sourcecode can be found at
http://www.johannes-bauer.com/crash/crash.c and my kernel configuration
is at http://www.johannes-bauer.com/crash/config
Any further questions: feel free to ask. Please CC me for any posts in
this thread.
Greetings,
Johannes
--
"A PC without Windows is like a chocolate cake without mustard."
Johannes Bauer
91054 Erlangen
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: x86_64 system lockup from userspace using setitimer() 2007-03-13 18:55 x86_64 system lockup from userspace using setitimer() Johannes Bauer @ 2007-03-13 19:19 ` Andreas Schwab 2007-03-13 20:02 ` Chuck Ebbert 1 sibling, 0 replies; 16+ messages in thread From: Andreas Schwab @ 2007-03-13 19:19 UTC (permalink / raw) To: Johannes Bauer; +Cc: linux-kernel Johannes Bauer <JohannesBauer@gmx.de> writes: > Dear Community, > > I think I've encountered a bug with the Linux kernel which results in a > complete system lockup and which can be started without root > priviliges. It's reproducible with 2.6.20.1 and 2.6.20.2 and only x64_64 > seems affected. I can also reproduce it on ia64 with 2.6.20. 2.6.16.42 is ok. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: x86_64 system lockup from userspace using setitimer() 2007-03-13 18:55 x86_64 system lockup from userspace using setitimer() Johannes Bauer 2007-03-13 19:19 ` Andreas Schwab @ 2007-03-13 20:02 ` Chuck Ebbert 2007-03-13 20:33 ` Thomas Gleixner 1 sibling, 1 reply; 16+ messages in thread From: Chuck Ebbert @ 2007-03-13 20:02 UTC (permalink / raw) To: Johannes Bauer; +Cc: linux-kernel, Thomas Gleixner, schwab Johannes Bauer wrote: > Dear Community, > > I think I've encountered a bug with the Linux kernel which results in a > complete system lockup and which can be started without root priviliges. > It's reproducible with 2.6.20.1 and 2.6.20.2 and only x64_64 seems > affected. > > Here's the code which triggers the bug (originally found by me using an > only partly initialized "struct itimerval" structure - hence the strange > values in it_interval): > > -----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<----- > #include <stdio.h> > #include <sys/time.h> > #include <unistd.h> > > int main(int argc, char **argv) { > struct itimerval tim = { > .it_interval = { > .tv_sec = 140735669863712, > .tv_usec = 4199521 > }, > .it_value = { > .tv_sec = 0, > .tv_usec = 100000 > } > }; > setitimer(ITIMER_REAL, &tim, NULL); > while (1) sleep(1); > return 0; > } > -----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<-----8<----- > > Compiled with gcc 4.1.1 with "gcc -O2 -Wall -o crash crash.c". > > The sourcecode can be found at > http://www.johannes-bauer.com/crash/crash.c and my kernel configuration > is at http://www.johannes-bauer.com/crash/config > > Any further questions: feel free to ask. Please CC me for any posts in > this thread. Could this be fixed by: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8bfd9a7a229b5f3d3eda5d7d45c2eebec5b4ba16 [PATCH] hrtimers: prevent possible itimer DoS ? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: x86_64 system lockup from userspace using setitimer() 2007-03-13 20:02 ` Chuck Ebbert @ 2007-03-13 20:33 ` Thomas Gleixner 2007-03-14 10:00 ` [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() Thomas Gleixner 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2007-03-13 20:33 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Johannes Bauer, linux-kernel, schwab On Tue, 2007-03-13 at 16:02 -0400, Chuck Ebbert wrote: > > struct itimerval tim = { > > .it_interval = { > > .tv_sec = 140735669863712, > > .tv_usec = 4199521 > > }, > Could this be fixed by: > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8bfd9a7a229b5f3d3eda5d7d45c2eebec5b4ba16 > > [PATCH] hrtimers: prevent possible itimer DoS No. The possible DoS is only when high res timers are enabled, which is not the case in 2.6.20. Looking at the values 140735669863712 = 0x7FFF 939C 0520 We convert second to nanoseconds: 140735669863712 * 1e9 = 0x1DCD 4BC3 6B82 914B 4000 The seconds value is limited to LONG_MAX, but on a 64 bit machine, the 140735669863712 is inside LONG_MAX and we have an multiplication overflow. I'm not sure, how this results in a DoS, but I will look into this tomorrow morning, when I'm more awake. tglx ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-13 20:33 ` Thomas Gleixner @ 2007-03-14 10:00 ` Thomas Gleixner 2007-03-14 10:08 ` Ingo Molnar ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Thomas Gleixner @ 2007-03-14 10:00 UTC (permalink / raw) To: Chuck Ebbert Cc: Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Andrew Morton, Ingo Molnar hrtimer_forward() does not check for the possible overflow of timer->expires. This can happen on 64 bit machines with large interval values and results currently in an endless loop in the softirq because the expiry value becomes negative and therefor the timer is expired all the time. Check for this condition and set the expiry value to the max. expiry time in the future. The fix should be applied to stable kernel series as well. Signed-off-by: Thomas Gleixner <tglx@linutronix,de> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index ec4cb9f..5e7122d 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -644,6 +644,12 @@ hrtimer_forward(struct hrtimer *timer, k orun++; } timer->expires = ktime_add(timer->expires, interval); + /* + * Make sure, that the result did not wrap with a very large + * interval. + */ + if (timer->expires.tv64 < 0) + timer->expires = ktime_set(KTIME_SEC_MAX, 0); return orun; } ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-14 10:00 ` [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() Thomas Gleixner @ 2007-03-14 10:08 ` Ingo Molnar 2007-03-16 20:43 ` Andrew Morton 2007-04-04 21:11 ` Adrian Bunk 2 siblings, 0 replies; 16+ messages in thread From: Ingo Molnar @ 2007-03-14 10:08 UTC (permalink / raw) To: Thomas Gleixner Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Andrew Morton * Thomas Gleixner <tglx@linutronix.de> wrote: > hrtimer_forward() does not check for the possible overflow of > timer->expires. This can happen on 64 bit machines with large interval > values and results currently in an endless loop in the softirq because > the expiry value becomes negative and therefor the timer is expired > all the time. > > Check for this condition and set the expiry value to the max. expiry > time in the future. > > The fix should be applied to stable kernel series as well. > > Signed-off-by: Thomas Gleixner <tglx@linutronix,de> ouch ... nice one. Acked-by: Ingo Molnar <mingo@elte.hu> Ingo ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-14 10:00 ` [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() Thomas Gleixner 2007-03-14 10:08 ` Ingo Molnar @ 2007-03-16 20:43 ` Andrew Morton 2007-03-16 21:05 ` Thomas Gleixner 2007-04-04 21:11 ` Adrian Bunk 2 siblings, 1 reply; 16+ messages in thread From: Andrew Morton @ 2007-03-16 20:43 UTC (permalink / raw) To: tglx Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner <tglx@linutronix.de> wrote: > rtimer_forward() does not check for the possible overflow of > timer->expires. This can happen on 64 bit machines with large interval > values and results currently in an endless loop in the softirq because > the expiry value becomes negative and therefor the timer is expired all > the time. > > Check for this condition and set the expiry value to the max. expiry > time in the future. > > The fix should be applied to stable kernel series as well. > > Signed-off-by: Thomas Gleixner <tglx@linutronix,de> > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > index ec4cb9f..5e7122d 100644 > --- a/kernel/hrtimer.c > +++ b/kernel/hrtimer.c > @@ -644,6 +644,12 @@ hrtimer_forward(struct hrtimer *timer, k > orun++; > } > timer->expires = ktime_add(timer->expires, interval); > + /* > + * Make sure, that the result did not wrap with a very large > + * interval. > + */ > + if (timer->expires.tv64 < 0) > + timer->expires = ktime_set(KTIME_SEC_MAX, 0); > > return orun; > } kernel/hrtimer.c: In function 'hrtimer_forward': kernel/hrtimer.c:652: warning: overflow in implicit constant conversion problem is, KTIME_SEC_MAX is 9,223,372,036 and ktime_set() takes a `long'. This? --- a/include/linux/ktime.h~ktime_set-fix-arg-type +++ a/include/linux/ktime.h @@ -72,13 +72,13 @@ typedef union { * * Return the ktime_t representation of the value */ -static inline ktime_t ktime_set(const long secs, const unsigned long nsecs) +static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs) { #if (BITS_PER_LONG == 64) if (unlikely(secs >= KTIME_SEC_MAX)) return (ktime_t){ .tv64 = KTIME_MAX }; #endif - return (ktime_t) { .tv64 = (s64)secs * NSEC_PER_SEC + (s64)nsecs }; + return (ktime_t) { .tv64 = secs * NSEC_PER_SEC + (s64)nsecs }; } /* Subtract two ktime_t variables. rem = lhs -rhs: */ _ I worry about that `secs >= KTIME_SEC_MAX' comparison in there, too. Both operands are signed. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-16 20:43 ` Andrew Morton @ 2007-03-16 21:05 ` Thomas Gleixner 2007-03-18 21:16 ` Chuck Ebbert 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2007-03-16 21:05 UTC (permalink / raw) To: Andrew Morton Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar On Fri, 2007-03-16 at 12:43 -0800, Andrew Morton wrote: > On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner <tglx@linutronix.de> wrote: > > > rtimer_forward() does not check for the possible overflow of > > timer->expires. This can happen on 64 bit machines with large interval > > values and results currently in an endless loop in the softirq because > > the expiry value becomes negative and therefor the timer is expired all > > the time. > > > > Check for this condition and set the expiry value to the max. expiry > > time in the future. > > > > The fix should be applied to stable kernel series as well. > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix,de> > > > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > > index ec4cb9f..5e7122d 100644 > > --- a/kernel/hrtimer.c > > +++ b/kernel/hrtimer.c > > @@ -644,6 +644,12 @@ hrtimer_forward(struct hrtimer *timer, k > > orun++; > > } > > timer->expires = ktime_add(timer->expires, interval); > > + /* > > + * Make sure, that the result did not wrap with a very large > > + * interval. > > + */ > > + if (timer->expires.tv64 < 0) > > + timer->expires = ktime_set(KTIME_SEC_MAX, 0); > > > > return orun; > > } > > kernel/hrtimer.c: In function 'hrtimer_forward': > kernel/hrtimer.c:652: warning: overflow in implicit constant conversion > > problem is, KTIME_SEC_MAX is 9,223,372,036 and ktime_set() takes a `long'. Stupid me :( > This? > > --- a/include/linux/ktime.h~ktime_set-fix-arg-type > +++ a/include/linux/ktime.h > @@ -72,13 +72,13 @@ typedef union { > * > * Return the ktime_t representation of the value > */ > -static inline ktime_t ktime_set(const long secs, const unsigned long nsecs) > +static inline ktime_t ktime_set(const s64 secs, const unsigned long nsecs) > { > #if (BITS_PER_LONG == 64) > if (unlikely(secs >= KTIME_SEC_MAX)) > return (ktime_t){ .tv64 = KTIME_MAX }; > #endif > - return (ktime_t) { .tv64 = (s64)secs * NSEC_PER_SEC + (s64)nsecs }; > + return (ktime_t) { .tv64 = secs * NSEC_PER_SEC + (s64)nsecs }; > } > > /* Subtract two ktime_t variables. rem = lhs -rhs: */ > _ > > I worry about that `secs >= KTIME_SEC_MAX' comparison in there, too. Both > operands are signed. I'd prefer this one: The maximum seconds value we can handle on 32bit is LONG_MAX. diff --git a/include/linux/ktime.h b/include/linux/ktime.h index c68c7ac..248305b 100644 --- a/include/linux/ktime.h +++ b/include/linux/ktime.h @@ -57,7 +57,11 @@ typedef union { } ktime_t; #define KTIME_MAX ((s64)~((u64)1 << 63)) -#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#if (BITS_PER_LONG == 64) +# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#else +# define KTIME_SEC_MAX LONG_MAX +#endif /* * ktime_t definitions when using the 64-bit scalar representation: ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-16 21:05 ` Thomas Gleixner @ 2007-03-18 21:16 ` Chuck Ebbert 2007-03-18 21:32 ` Thomas Gleixner 0 siblings, 1 reply; 16+ messages in thread From: Chuck Ebbert @ 2007-03-18 21:16 UTC (permalink / raw) To: tglx Cc: Andrew Morton, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar Thomas Gleixner wrote: > > I'd prefer this one: The maximum seconds value we can handle on 32bit is > LONG_MAX. > > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > index c68c7ac..248305b 100644 > --- a/include/linux/ktime.h > +++ b/include/linux/ktime.h > @@ -57,7 +57,11 @@ typedef union { > } ktime_t; > > #define KTIME_MAX ((s64)~((u64)1 << 63)) > -#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) > +#if (BITS_PER_LONG == 64) > +# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) > +#else > +# define KTIME_SEC_MAX LONG_MAX > +#endif > > /* > * ktime_t definitions when using the 64-bit scalar representation: > Just to be clear: this replaces the earlier patch, right? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-18 21:16 ` Chuck Ebbert @ 2007-03-18 21:32 ` Thomas Gleixner 2007-03-18 21:53 ` Chuck Ebbert 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2007-03-18 21:32 UTC (permalink / raw) To: Chuck Ebbert Cc: Andrew Morton, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar On Sun, 2007-03-18 at 17:16 -0400, Chuck Ebbert wrote: > Thomas Gleixner wrote: > > > > I'd prefer this one: The maximum seconds value we can handle on 32bit is > > LONG_MAX. > > > > diff --git a/include/linux/ktime.h b/include/linux/ktime.h > > index c68c7ac..248305b 100644 > > --- a/include/linux/ktime.h > > +++ b/include/linux/ktime.h > > @@ -57,7 +57,11 @@ typedef union { > > } ktime_t; > > > > #define KTIME_MAX ((s64)~((u64)1 << 63)) > > -#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) > > +#if (BITS_PER_LONG == 64) > > +# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) > > +#else > > +# define KTIME_SEC_MAX LONG_MAX > > +#endif > > > > /* > > * ktime_t definitions when using the 64-bit scalar representation: > > > > Just to be clear: this replaces the earlier patch, right? This replaces the fix Andrew did. http://marc.info/?l=linux-kernel&m=117407812411997&w=2 tglx ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-18 21:32 ` Thomas Gleixner @ 2007-03-18 21:53 ` Chuck Ebbert 2007-03-18 22:04 ` Thomas Gleixner 0 siblings, 1 reply; 16+ messages in thread From: Chuck Ebbert @ 2007-03-18 21:53 UTC (permalink / raw) To: tglx Cc: Andrew Morton, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar Thomas Gleixner wrote: > On Sun, 2007-03-18 at 17:16 -0400, Chuck Ebbert wrote: >> Thomas Gleixner wrote: >>> I'd prefer this one: The maximum seconds value we can handle on 32bit is >>> LONG_MAX. >>> >>> diff --git a/include/linux/ktime.h b/include/linux/ktime.h >>> index c68c7ac..248305b 100644 >>> --- a/include/linux/ktime.h >>> +++ b/include/linux/ktime.h >>> @@ -57,7 +57,11 @@ typedef union { >>> } ktime_t; >>> >>> #define KTIME_MAX ((s64)~((u64)1 << 63)) >>> -#define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) >>> +#if (BITS_PER_LONG == 64) >>> +# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) >>> +#else >>> +# define KTIME_SEC_MAX LONG_MAX >>> +#endif >>> >>> /* >>> * ktime_t definitions when using the 64-bit scalar representation: >>> >> Just to be clear: this replaces the earlier patch, right? > > This replaces the fix Andrew did. > > http://marc.info/?l=linux-kernel&m=117407812411997&w=2 > Right, but is the original "Prevent DOS" patch from you still needed? Or did Andrew's patch replace that one, and now this replaces his? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-18 21:53 ` Chuck Ebbert @ 2007-03-18 22:04 ` Thomas Gleixner 2007-03-18 22:02 ` Chuck Ebbert 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2007-03-18 22:04 UTC (permalink / raw) To: Chuck Ebbert Cc: Andrew Morton, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar On Sun, 2007-03-18 at 17:53 -0400, Chuck Ebbert wrote: > >> Just to be clear: this replaces the earlier patch, right? > > > > This replaces the fix Andrew did. > > > > http://marc.info/?l=linux-kernel&m=117407812411997&w=2 > > > > Right, but is the original "Prevent DOS" patch from you still needed? > Or did Andrew's patch replace that one, and now this replaces his? The original patch is still needed - it handles the problem in the first place. I missed to compile it for 32bit and Andrew did a fix, which I replaced. tglx ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-18 22:04 ` Thomas Gleixner @ 2007-03-18 22:02 ` Chuck Ebbert 0 siblings, 0 replies; 16+ messages in thread From: Chuck Ebbert @ 2007-03-18 22:02 UTC (permalink / raw) To: tglx Cc: Andrew Morton, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Adrian Bunk, Ingo Molnar Thomas Gleixner wrote: > On Sun, 2007-03-18 at 17:53 -0400, Chuck Ebbert wrote: >>>> Just to be clear: this replaces the earlier patch, right? >>> This replaces the fix Andrew did. >>> >>> http://marc.info/?l=linux-kernel&m=117407812411997&w=2 >>> >> Right, but is the original "Prevent DOS" patch from you still needed? >> Or did Andrew's patch replace that one, and now this replaces his? > > The original patch is still needed - it handles the problem in the first > place. > > I missed to compile it for 32bit and Andrew did a fix, which I replaced. Ah, OK, and both of those are now in the queue for 2.6.20-stable. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-03-14 10:00 ` [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() Thomas Gleixner 2007-03-14 10:08 ` Ingo Molnar 2007-03-16 20:43 ` Andrew Morton @ 2007-04-04 21:11 ` Adrian Bunk 2007-04-04 21:30 ` Thomas Gleixner 2 siblings, 1 reply; 16+ messages in thread From: Adrian Bunk @ 2007-04-04 21:11 UTC (permalink / raw) To: Thomas Gleixner Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Andrew Morton, Ingo Molnar On Wed, Mar 14, 2007 at 11:00:12AM +0100, Thomas Gleixner wrote: > hrtimer_forward() does not check for the possible overflow of > timer->expires. This can happen on 64 bit machines with large interval > values and results currently in an endless loop in the softirq because > the expiry value becomes negative and therefor the timer is expired all > the time. > > Check for this condition and set the expiry value to the max. expiry > time in the future. > > The fix should be applied to stable kernel series as well. Is this relevant for 2.6.16? I'm asking since KTIME_SEC_MAX is not used in 2.6.16, and therefore the check in ktime_set() is also missing. > Signed-off-by: Thomas Gleixner <tglx@linutronix,de> > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > index ec4cb9f..5e7122d 100644 > --- a/kernel/hrtimer.c > +++ b/kernel/hrtimer.c > @@ -644,6 +644,12 @@ hrtimer_forward(struct hrtimer *timer, k > orun++; > } > timer->expires = ktime_add(timer->expires, interval); > + /* > + * Make sure, that the result did not wrap with a very large > + * interval. > + */ > + if (timer->expires.tv64 < 0) > + timer->expires = ktime_set(KTIME_SEC_MAX, 0); > > return orun; > } > cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-04-04 21:11 ` Adrian Bunk @ 2007-04-04 21:30 ` Thomas Gleixner 2007-04-09 13:01 ` Adrian Bunk 0 siblings, 1 reply; 16+ messages in thread From: Thomas Gleixner @ 2007-04-04 21:30 UTC (permalink / raw) To: Adrian Bunk Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Andrew Morton, Ingo Molnar On Wed, 2007-04-04 at 23:11 +0200, Adrian Bunk wrote: > On Wed, Mar 14, 2007 at 11:00:12AM +0100, Thomas Gleixner wrote: > > hrtimer_forward() does not check for the possible overflow of > > timer->expires. This can happen on 64 bit machines with large interval > > values and results currently in an endless loop in the softirq because > > the expiry value becomes negative and therefor the timer is expired all > > the time. > > > > Check for this condition and set the expiry value to the max. expiry > > time in the future. > > > > The fix should be applied to stable kernel series as well. > > > Is this relevant for 2.6.16? > > I'm asking since KTIME_SEC_MAX is not used in 2.6.16, and therefore the > check in ktime_set() is also missing. KTIME_SEC_MAX was introduced with commit 96dd7421a06a5bc6eb731323b95efcb2fd864854 to fix a conversion problem on 64 bit machines, which is also present in 2.6.16 AFAICT. The patch just makes use of this constant. So you need to pull it as well. tglx ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() 2007-04-04 21:30 ` Thomas Gleixner @ 2007-04-09 13:01 ` Adrian Bunk 0 siblings, 0 replies; 16+ messages in thread From: Adrian Bunk @ 2007-04-09 13:01 UTC (permalink / raw) To: Thomas Gleixner Cc: Chuck Ebbert, Johannes Bauer, linux-kernel, schwab, Stable Kernel Team, Greg KH, Andrew Morton, Ingo Molnar On Wed, Apr 04, 2007 at 11:30:48PM +0200, Thomas Gleixner wrote: > On Wed, 2007-04-04 at 23:11 +0200, Adrian Bunk wrote: > > On Wed, Mar 14, 2007 at 11:00:12AM +0100, Thomas Gleixner wrote: > > > hrtimer_forward() does not check for the possible overflow of > > > timer->expires. This can happen on 64 bit machines with large interval > > > values and results currently in an endless loop in the softirq because > > > the expiry value becomes negative and therefor the timer is expired all > > > the time. > > > > > > Check for this condition and set the expiry value to the max. expiry > > > time in the future. > > > > > > The fix should be applied to stable kernel series as well. > > > > > > Is this relevant for 2.6.16? > > > > I'm asking since KTIME_SEC_MAX is not used in 2.6.16, and therefore the > > check in ktime_set() is also missing. > > KTIME_SEC_MAX was introduced with commit > 96dd7421a06a5bc6eb731323b95efcb2fd864854 > > to fix a conversion problem on 64 bit machines, which is also present in > 2.6.16 AFAICT. > > The patch just makes use of this constant. So you need to pull it as > well. Thanks, below is what I applied. > tglx cu Adrian Thomas Gleixner (3): prevent timespec/timeval to ktime_t overflow fix MTIME_SEC_MAX on 32-bit hrtimer: prevent overrun DoS in hrtimer_forward() diff --git a/include/linux/ktime.h b/include/linux/ktime.h index f3dec45..4548ddb 100644 --- a/include/linux/ktime.h +++ b/include/linux/ktime.h @@ -56,7 +56,12 @@ typedef union { #endif } ktime_t; -#define KTIME_MAX (~((u64)1 << 63)) +#define KTIME_MAX ((s64)~((u64)1 << 63)) +#if (BITS_PER_LONG == 64) +# define KTIME_SEC_MAX (KTIME_MAX / NSEC_PER_SEC) +#else +# define KTIME_SEC_MAX LONG_MAX +#endif /* * ktime_t definitions when using the 64-bit scalar representation: @@ -77,6 +82,10 @@ typedef union { */ static inline ktime_t ktime_set(const long secs, const unsigned long nsecs) { +#if (BITS_PER_LONG == 64) + if (unlikely(secs >= KTIME_SEC_MAX)) + return (ktime_t){ .tv64 = KTIME_MAX }; +#endif return (ktime_t) { .tv64 = (s64)secs * NSEC_PER_SEC + (s64)nsecs }; } diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 14bc9cf..a29ceb0 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -316,6 +316,12 @@ hrtimer_forward(struct hrtimer *timer, ktime_t interval) orun++; } timer->expires = ktime_add(timer->expires, interval); + /* + * Make sure, that the result did not wrap with a very large + * interval. + */ + if (timer->expires.tv64 < 0) + timer->expires = ktime_set(KTIME_SEC_MAX, 0); return orun; } ^ permalink raw reply related [flat|nested] 16+ messages in thread
end of thread, other threads:[~2007-04-09 13:01 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-03-13 18:55 x86_64 system lockup from userspace using setitimer() Johannes Bauer 2007-03-13 19:19 ` Andreas Schwab 2007-03-13 20:02 ` Chuck Ebbert 2007-03-13 20:33 ` Thomas Gleixner 2007-03-14 10:00 ` [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward() Thomas Gleixner 2007-03-14 10:08 ` Ingo Molnar 2007-03-16 20:43 ` Andrew Morton 2007-03-16 21:05 ` Thomas Gleixner 2007-03-18 21:16 ` Chuck Ebbert 2007-03-18 21:32 ` Thomas Gleixner 2007-03-18 21:53 ` Chuck Ebbert 2007-03-18 22:04 ` Thomas Gleixner 2007-03-18 22:02 ` Chuck Ebbert 2007-04-04 21:11 ` Adrian Bunk 2007-04-04 21:30 ` Thomas Gleixner 2007-04-09 13:01 ` Adrian Bunk
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.