xorl %eax, %eax

CVE-2007-5966: Linux kernel hrtimer_start Signedness Issue

with 2 comments

This is another old but neat bug which was reported by Thomas Gleixner according to 2.6.23.10’s ChangeLog file. This issue affects Linux kernel prior to 2.6.23.10 as you can easily deduce and the susceptible code resides at kernel/hrtimer.c. Here is hrtimer_start() function from v2.6.23 Linux kernel:

/**
 * hrtimer_start - (re)start an relative timer on the current CPU
 * @timer:      the timer to be added
 * @tim:        expiry time
 * @mode:       expiry mode: absolute (HRTIMER_ABS) or relative (HRTIMER_REL)
 *
 * Returns:
 *  0 on success
 *  1 when the timer was active
 */
int
hrtimer_start(struct hrtimer *timer, ktime_t tim, const enum hrtimer_mode mode)
{
        struct hrtimer_clock_base *base, *new_base;
        unsigned long flags;
        int ret;
    ...
        /* Switch the timer base, if necessary: */
        new_base = switch_hrtimer_base(timer, base);

        if (mode == HRTIMER_MODE_REL) {
                tim = ktime_add(tim, new_base->get_time());
                /*
                 * CONFIG_TIME_LOW_RES is a temporary way for architectures
                 * to signal that they simply return xtime in
                 * do_gettimeoffset(). In this case we want to round up by
                 * resolution when starting a relative timer, to avoid short
                 * timeouts. This will go away with the GTOD framework.
                 */
#ifdef CONFIG_TIME_LOW_RES
                tim = ktime_add(tim, base->resolution);
#endif
        }
        timer->expires = tim;
     ...
        return ret;
}
EXPORT_SYMBOL_GPL(hrtimer_start);

As you can see, this is an exported routine which is used to restart a (high-resolution) timer on the current processor. If ‘mode’ is set to HRTIMER_MODE_REL which means that value of time is relative to now it will update ‘tim’ using ktime_add() macro to add the two ktime variables ‘tim’ and ‘new_base->get_time()’. Now, we don’t care about CONFIG_TIME_LOW_RES so we are out of the if clause. As you can see, it updates timer->expires which is the absolute expiry time of the hrtimer to ‘tim’. This seem to be correct but if we have a look at hrtimer structure for the data type of timer->expires we’ll see this:

struct hrtimer {
        struct rb_node                  node;
        ktime_t                         expires;
        enum hrtimer_restart            (*function)(struct hrtimer *);
        struct hrtimer_clock_base       *base;
        unsigned long                   state;
#ifdef CONFIG_HIGH_RES_TIMERS
        enum hrtimer_cb_mode            cb_mode;
        struct list_head                cb_entry;
#endif
#ifdef CONFIG_TIMER_STATS
        void                            *start_site;
        char                            start_comm[16];
        int                             start_pid;
#endif
};

Which can be found at include/linux/hrtimer.h and ktime_t is located at include/linux/ktime.h like this:

union ktime {
        s64     tv64;
#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR)
        struct {
# ifdef __BIG_ENDIAN
        s32     sec, nsec;
# else
        s32     nsec, sec;
# endif
        } tv;
#endif
};

typedef union ktime ktime_t;            /* Kill this */

Now, since timer->expires is simply a signed integer, if the user requests a large relative timeout value for ‘tim’, this could result in a negative value. As we can read at the rPath report:

This in turn is causing the clockevents_set_next() function to set an
huge timeout and sleep for quite a long time when we have a clock
source which is capable of long sleeps like HPET. With PIT this almost
goes unnoticed as the maximum delta is ~27ms. The non-hrt/nohz code
sorts this out in the next timer interrupt, so we never noticed that
problem which has been there since the first day of hrtimers.

The bug was fixed by applying the following patch:

#endif
+                /*
+                 * Careful here: User space might have asked for a
+                 * very long sleep, so the add above might result in a
+                 * negative number, which enqueues the timer in front
+                 * of the queue.
+                 */
+                if (tim.tv64 < 0)
+                        tim.tv64 = KTIME_MAX;
        }
        timer->expires = tim;

Written by xorl

August 6, 2009 at 13:02

Posted in bugs, linux

2 Responses

Subscribe to comments with RSS.

  1. Hi

    I have a callback function for hrtimer that
    only waits for the first iteration of the timing loop.

    Do I need to re-prime the timer ?

    I send HRTIMER_RESTART back in the calling function
    and what happens next is the linux system hangs.

    If I put a guard in to make the callback function
    send back HRTIMER_NORESTART, it calls the int
    as many times as the guard allows but the timer is ignored.
    That is it calls the callback as fast as it can.

    DO i need to re-init the timer in the callback function somehow ?

    Robin

    November 11, 2010 at 17:23

  2. @Robin: well, your question is off topic and I don’t know the answer. Try asking some linux developer. :)

    xorl

    November 11, 2010 at 21:03


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s