Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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 <[EMAIL PROTECTED],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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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 [EMAIL PROTECTED],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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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=117407812411997=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. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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=117407812411997=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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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=117407812411997=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?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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=117407812411997=2
tglx
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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-kernelm=117407812411997w=2
tglx
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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-kernelm=117407812411997w=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?
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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-kernelm=117407812411997w=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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
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-kernelm=117407812411997w=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. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
On Fri, 2007-03-16 at 12:43 -0800, Andrew Morton wrote:
> On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED],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:
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED],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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner [EMAIL PROTECTED] 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 [EMAIL PROTECTED],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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
On Fri, 2007-03-16 at 12:43 -0800, Andrew Morton wrote:
On Wed, 14 Mar 2007 11:00:12 +0100 Thomas Gleixner [EMAIL PROTECTED] 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 [EMAIL PROTECTED],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:
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
* Thomas Gleixner <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED],de> ouch ... nice one. Acked-by: Ingo Molnar <[EMAIL PROTECTED]> Ingo - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] hrtimer: prevent overrun DoS in hrtimer_forward()
* Thomas Gleixner [EMAIL PROTECTED] 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 [EMAIL PROTECTED],de ouch ... nice one. Acked-by: Ingo Molnar [EMAIL PROTECTED] Ingo - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
