2.4.20 kernel/timer.c may incorrectly reenable interrupts

[Keith Owens]

2.4.20 kernel/timer.c

static inline void update_times(void)
{
	unsigned long ticks;

	/*
	 * update_times() is run from the raw timer_bh handler so we
	 * just know that the irqs are locally enabled and so we don't
	 * need to save/restore the flags of the local CPU here. -arca
	 */
	write_lock_irq(&xtime_lock);
	vxtime_lock();

	ticks = jiffies - wall_jiffies;
	if (ticks) {
		wall_jiffies += ticks;
		update_wall_time(ticks);
	}
	vxtime_unlock();
	write_unlock_irq(&xtime_lock);
	calc_load(ticks);
}

I hit one case when the routine was called with interrupts disabled and
it unconditionally enabled them, with nasty side effects.  Code fragment

  local_irq_save();
  local_bh_disable();
  ....
  local_bh_enable();
  local_irq_restore();

local_bh_enable() checks for pending softirqs, finds that there is an
outstanding timer bh and runs it.  do_softirq() -> tasklet_hi_action()
-> bh_action() -> timer_bh() -> update_times() which unconditionally
reenables interrupts.  Then the timer code issued cli(), because
interrupts were incorrectly reenabled it tried to get the global cli
lock and hung.

There is no documentation that defines the required nesting order of
local_irq and local_bh.  Even if the above code fragment is deemed to
be illegal, there are uses of local_bh_enable() all through the kernel,
it will be difficult to prove that none of them are called with
interrupts disabled.  If there is any chance that local_bh_enable() is
called with interrupts off, update_times() is wrong.