Skip to content

Commit

Permalink
tick: Make oneshot broadcast robust vs. CPU offlining
Browse files Browse the repository at this point in the history
In periodic mode we remove offline cpus from the broadcast propagation
mask. In oneshot mode we fail to do so. This was not a problem so far,
but the recent changes to the broadcast propagation introduced a
constellation which can result in a NULL pointer dereference.

What happens is:

CPU0			CPU1
			idle()
			  arch_idle()
			    tick_broadcast_oneshot_control(OFF);
			      set cpu1 in tick_broadcast_force_mask
			  if (cpu_offline())
			     arch_cpu_dead()

cpu_dead_cleanup(cpu1)
 cpu1 tickdevice pointer = NULL

broadcast interrupt
  dereference cpu1 tickdevice pointer -> OOPS

We dereference the pointer because cpu1 is still set in
tick_broadcast_force_mask and tick_do_broadcast() expects a valid
cpumask and therefor lacks any further checks.

Remove the cpu from the tick_broadcast_force_mask before we set the
tick device pointer to NULL. Also add a sanity check to the oneshot
broadcast function, so we can detect such issues w/o crashing the
machine.

Reported-by: Prarit Bhargava <prarit@redhat.com>
Cc: athorlton@sgi.com
Cc: CAI Qian <caiqian@redhat.com>
Link: http://lkml.kernel.org/r/alpine.DEB.2.02.1306261303260.4013@ionos.tec.linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
  • Loading branch information
Thomas Gleixner committed Jul 2, 2013
1 parent 47433b8 commit c9b5a26
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions kernel/time/tick-broadcast.c
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,13 @@ static void tick_handle_oneshot_broadcast(struct clock_event_device *dev)
cpumask_or(tmpmask, tmpmask, tick_broadcast_force_mask);
cpumask_clear(tick_broadcast_force_mask);

/*
* Sanity check. Catch the case where we try to broadcast to
* offline cpus.
*/
if (WARN_ON_ONCE(!cpumask_subset(tmpmask, cpu_online_mask)))
cpumask_and(tmpmask, tmpmask, cpu_online_mask);

/*
* Wakeup the cpus which have an expired event.
*/
Expand Down Expand Up @@ -773,10 +780,12 @@ void tick_shutdown_broadcast_oneshot(unsigned int *cpup)
raw_spin_lock_irqsave(&tick_broadcast_lock, flags);

/*
* Clear the broadcast mask flag for the dead cpu, but do not
* stop the broadcast device!
* Clear the broadcast masks for the dead cpu, but do not stop
* the broadcast device!
*/
cpumask_clear_cpu(cpu, tick_broadcast_oneshot_mask);
cpumask_clear_cpu(cpu, tick_broadcast_pending_mask);
cpumask_clear_cpu(cpu, tick_broadcast_force_mask);

raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);
}
Expand Down

0 comments on commit c9b5a26

Please sign in to comment.