[Kernel] Potential bug for using jiffies - ex. schedule_timeout() - at Linux kernel.

Domain/Kernel 2011.08.18 08:29

There is function for sleep in kernel - ex. msleep(). And schedule_timeout() is used in those.
And, schedule_timeout() calculates time for expiration based on current jiffies.
But some functions disable local irq or irq for a while in it.
For example, printk does this - disabling local irq.
But, jiffies is updated based on local timer irq (In case of SMP, one defined core - usually cpu0 - is used.)
So, disabling local irq - for timer core - may prevent system from updating jiffies.
At this moment, calling schedule_timeout() leads to set timer which expiring time is earlier than it should be because jiffies are behind of real time.
So, for example, msleep(100) may wake up after 50ms at this scenario.
This is very dangerous.
But, actually, lots of code uses jiffies directly in kernel.
So, I'm not sure that all those codes are safe in case that jiffies is behind.
Anyway, here is my sample fix for this potential issue of schedule_timeout().
I couldn't find any fix regarding this issue even in kernel-3.0.
So, I'm not sure I missed something.
But, as for me, this is definitely issue to consider when variable jiffies is directly used.
I hope that my sample fix regarding schedule_timeout is helpful for others.
(In my opinion, fundamentally, all codes that uses jiffies directly should be re-examined whether it is really safe or not in case that jiffies is behind.)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 6811f4b..6c89958 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -290,6 +290,8 @@ extern unsigned long preset_lpj;

 #endif

+extern unsigned long exact_jiffies(void);
+
 /*
  * Convert various time units to each other:
  */
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 56f87fa..37e7bbf 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -99,6 +99,32 @@ static ktime_t tick_init_jiffy_update(void)
        return period;
 }

+/**
+ * exact_jiffies - return real jiffies value
+ *
+ * You can't sure that value from %jiffies varaible is real current time.
+ * Jiffies may not be updated for a while due to several reasones.
+ * So, to get exact value, current ktime and %last_jiffies_update should be used
+ */
+unsigned long exact_jiffies(void)
+{
+       unsigned long exact = jiffies;
+       if (tick_period.tv64
+           && last_jiffies_update.tv64) {
+               unsigned long seq, ticks;
+               ktime_t delta;
+               do {
+                       seq = read_seqbegin(&xtime_lock);
+                       delta = ktime_sub(ktime_get(), last_jiffies_update);
+                       ticks = ktime_divns(delta, ktime_to_ns(tick_period));
+                       /* +1 to compensate loss at division */
+                       exact = jiffies + ticks + 1;
+               } while (read_seqretry(&xtime_lock, seq));
+       }
+       return exact;
+}
+EXPORT_SYMBOL_GPL(exact_jiffies);
+
 /*
  * NOHZ - aka dynamic tick functionality
  */
diff --git a/kernel/timer.c b/kernel/timer.c
index c4714a6..628a714 100644
--- a/kernel/timer.c
+++ b/kernel/timer.c
@@ -1304,7 +1304,6 @@ void run_local_timers(void)
  * without sampling the sequence number in xtime_lock.
  * jiffies is defined in the linker script...
  */
-
 void do_timer(unsigned long ticks)
 {
        jiffies_64 += ticks;
@@ -1457,7 +1456,7 @@ signed long __sched schedule_timeout(signed long timeout)
                }
        }

-       expire = timeout + jiffies;
+       expire = timeout + exact_jiffies();

        setup_timer_on_stack(&timer, process_timeout, (unsigned long)current);
        __mod_timer(&timer, expire, false, TIMER_NOT_PINNED);
@@ -1467,6 +1466,13 @@ signed long __sched schedule_timeout(signed long timeout)
        /* Remove the timer from the object tracker */
        destroy_timer_on_stack(&timer);

+       /*
+        * Reaching here means "timer interrupt is issued
+        *   and 'jiffies' is updated."
+        * So, 'jiffies' here is recently-updated-value
+        *   and 'jiffies' can be directly used
+        *   instead of using 'exact_jiffies()'
+        */
        timeout = expire - jiffies;

  out:

done.

신고
Trackback 0 : Comment 0