Skip to content

Commit 2389c6e

Browse files
committed
posix-timers: Ensure that timer initialization is fully visible
Frederic pointed out that the memory operations to initialize the timer are not guaranteed to be visible, when __lock_timer() observes timer::it_signal valid under timer::it_lock: T0 T1 --------- ----------- do_timer_create() // A new_timer->.... = .... spin_lock(current->sighand) // B WRITE_ONCE(new_timer->it_signal, current->signal) spin_unlock(current->sighand) sys_timer_*() t = __lock_timer() spin_lock(&timr->it_lock) // observes B if (timr->it_signal == current->signal) return timr; if (!t) return; // Is not guaranteed to observe A Protect the write of timer::it_signal, which makes the timer valid, with timer::it_lock as well. This guarantees that T1 must observe the initialization A completely, when it observes the valid signal pointer under timer::it_lock. sighand::siglock must still be taken to protect the signal::posix_timers list. Reported-by: Frederic Weisbecker <[email protected]> Suggested-by: Frederic Weisbecker <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Reviewed-by: Frederic Weisbecker <[email protected]> Link: https://lore.kernel.org/all/[email protected]
1 parent fc661d0 commit 2389c6e

File tree

1 file changed

+14
-7
lines changed

1 file changed

+14
-7
lines changed

kernel/time/posix-timers.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -462,14 +462,21 @@ static int do_timer_create(clockid_t which_clock, struct sigevent *event,
462462
if (error)
463463
goto out;
464464

465-
spin_lock_irq(&current->sighand->siglock);
466-
/* This makes the timer valid in the hash table */
467-
WRITE_ONCE(new_timer->it_signal, current->signal);
468-
hlist_add_head(&new_timer->list, &current->signal->posix_timers);
469-
spin_unlock_irq(&current->sighand->siglock);
470465
/*
471-
* After unlocking sighand::siglock @new_timer is subject to
472-
* concurrent removal and cannot be touched anymore
466+
* timer::it_lock ensures that __lock_timer() observes a fully
467+
* initialized timer when it observes a valid timer::it_signal.
468+
*
469+
* sighand::siglock is required to protect signal::posix_timers.
470+
*/
471+
scoped_guard (spinlock_irq, &new_timer->it_lock) {
472+
guard(spinlock)(&current->sighand->siglock);
473+
/* This makes the timer valid in the hash table */
474+
WRITE_ONCE(new_timer->it_signal, current->signal);
475+
hlist_add_head(&new_timer->list, &current->signal->posix_timers);
476+
}
477+
/*
478+
* After unlocking @new_timer is subject to concurrent removal and
479+
* cannot be touched anymore
473480
*/
474481
return 0;
475482
out:

0 commit comments

Comments
 (0)