[PATCH] posix-timers: fix posix_cpu_timer_set() vs run_posix_cpu_timers() race
This might be harmless, but looks like a race from code inspection (I
was unable to trigger it). I must admit, I don't understand why we
can't return TIMER_RETRY after 'spin_unlock(&p->sighand->siglock)'
without doing bump_cpu_timer(), but this is what original code does.
posix_cpu_timer_set:
read_lock(&tasklist_lock);
spin_lock(&p->sighand->siglock);
list_del_init(&timer->it.cpu.entry);
spin_unlock(&p->sighand->siglock);
We are probaly deleting the timer from run_posix_cpu_timers's 'firing'
local list_head while run_posix_cpu_timers() does list_for_each_safe.
Various bad things can happen, for example we can just delete this timer
so that list_for_each() will not notice it and run_posix_cpu_timers()
will not reset '->firing' flag. In that case,
....
if (timer->it.cpu.firing) {
read_unlock(&tasklist_lock);
timer->it.cpu.firing = -1;
return TIMER_RETRY;
}
sys_timer_settime() goes to 'retry:', calls posix_cpu_timer_set() again,
it returns TIMER_RETRY ...
Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
This commit is contained in:
committed by
Linus Torvalds
parent
ca531a0a5e
commit
a69ac4a78d
@@ -730,9 +730,15 @@ int posix_cpu_timer_set(struct k_itimer *timer, int flags,
|
|||||||
* Disarm any old timer after extracting its expiry time.
|
* Disarm any old timer after extracting its expiry time.
|
||||||
*/
|
*/
|
||||||
BUG_ON(!irqs_disabled());
|
BUG_ON(!irqs_disabled());
|
||||||
|
|
||||||
|
ret = 0;
|
||||||
spin_lock(&p->sighand->siglock);
|
spin_lock(&p->sighand->siglock);
|
||||||
old_expires = timer->it.cpu.expires;
|
old_expires = timer->it.cpu.expires;
|
||||||
list_del_init(&timer->it.cpu.entry);
|
if (unlikely(timer->it.cpu.firing)) {
|
||||||
|
timer->it.cpu.firing = -1;
|
||||||
|
ret = TIMER_RETRY;
|
||||||
|
} else
|
||||||
|
list_del_init(&timer->it.cpu.entry);
|
||||||
spin_unlock(&p->sighand->siglock);
|
spin_unlock(&p->sighand->siglock);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -780,7 +786,7 @@ int posix_cpu_timer_set(struct k_itimer *timer, int flags,
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (unlikely(timer->it.cpu.firing)) {
|
if (unlikely(ret)) {
|
||||||
/*
|
/*
|
||||||
* We are colliding with the timer actually firing.
|
* We are colliding with the timer actually firing.
|
||||||
* Punt after filling in the timer's old value, and
|
* Punt after filling in the timer's old value, and
|
||||||
@@ -788,8 +794,6 @@ int posix_cpu_timer_set(struct k_itimer *timer, int flags,
|
|||||||
* it as an overrun (thanks to bump_cpu_timer above).
|
* it as an overrun (thanks to bump_cpu_timer above).
|
||||||
*/
|
*/
|
||||||
read_unlock(&tasklist_lock);
|
read_unlock(&tasklist_lock);
|
||||||
timer->it.cpu.firing = -1;
|
|
||||||
ret = TIMER_RETRY;
|
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user