Race condition in time/alarmtimer.c

From: Marcus Gelderie
Date: Mon Jun 24 2013 - 15:12:36 EST


Hi,

there seems to be a race condition in kernel/time/alarmtimer.c

More specifically, the following function (line numbers correspond to actual file):

584 static int alarmtimer_do_nsleep(struct alarm *alarm, ktime_t absexp)
585 {
586 alarm->data = (void *)current;
587 do {
588 set_current_state(TASK_INTERRUPTIBLE);
589 alarm_start(alarm, absexp);
590 if (likely(alarm->data))
591 schedule();
592
593 alarm_cancel(alarm);
594 } while (alarm->data && !signal_pending(current));
595
596 __set_current_state(TASK_RUNNING);
597
598 return (alarm->data == NULL);
599 }

has a race: If the task is preempted after set_current_state(TASK_INTERRUPTIBLE)
but before the alarm is started in the next line, the task never wakes up.

Swapping both lines is not an option either, because then the alarm might trigger before
the thread sets itself to TASK_INTERRUPTIBLE, thereby loosing the wakeup.

A spinlock would disable preemption and protect alarm->data against the race from another CPU.
We could wrap lines 588 and 589 with a spin lock. Then the wakeup code would also aquire the
lock, of course. The lock could be attached to struct alarm.

An alternative would be a waitqueue, of course.

If folks agree with me, I will provide a patch.


Cheers
Marcus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/