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
-

[ message continues ]

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."
-

[ message continues ]

Could this be fixed by:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8bfd9a...

[PATCH] hrtimers: prevent possible itimer DoS

?

-

[ message continues ]

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


-

[ message continues ]

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


-

[ message continues ]

ouch ... nice one.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo
-

[ message continues ]

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.


-

[ message continues ]

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:


-

[ message continues ]

Just to be clear: this replaces the earlier patch, right?




-

[ message continues ]

This replaces the fix Andrew did.

http://marc.info/?l=linux-kernel&m=117407812411997&w=2

	tglx


-

[ message continues ]

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?

-

[ message continues ]

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


-

[ message continues ]

Ah, OK, and both of those are now in the queue for 2.6.20-stable.


-

[ message continues ]

Is this relevant for 2.6.16?

I'm asking since KTIME_SEC_MAX is not used in 2.6.16, and therefore the 

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

-

[ message continues ]

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


-

[ message continues ]

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

-

[ message continues ]