Commit 9d9e5220 authored by Thomas Gleixner's avatar Thomas Gleixner
Browse files

posix-timers: Prevent RT livelock in itimer_delete() 



itimer_delete() has a retry loop when the timer is concurrently expired. On
non-RT kernels this just spin-waits until the timer callback has completed,
except for posix CPU timers which have HAVE_POSIX_CPU_TIMERS_TASK_WORK
enabled.

In that case and on RT kernels the existing task could live lock when
preempting the task which does the timer delivery.

Replace spin_unlock() with an invocation of timer_wait_running() to handle
it the same way as the other retry loops in the posix timer code.

Fixes: ec8f954a ("posix-timers: Use a callback for cancel synchronization on PREEMPT_RT")
Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
Reviewed-by: default avatarFrederic Weisbecker <frederic@kernel.org>
Link: https://lore.kernel.org/r/87v8g7c50d.ffs@tglx
parent b7a7ce1b

kernel/time/posix-timers.c

+35 −8
Original line number Diff line number Diff line
@@ -1037,27 +1037,52 @@ SYSCALL_DEFINE1(timer_delete, timer_t, timer_id) 
}



/*

 * return timer owned by the process, used by exit_itimers

 * Delete a timer if it is armed, remove it from the hash and schedule it

 * for RCU freeing.

 */

static void itimer_delete(struct k_itimer *timer)

{

retry_delete:

	spin_lock_irq(&timer->it_lock);

	unsigned long flags;



	/*

	 * irqsave is required to make timer_wait_running() work.

	 */

	spin_lock_irqsave(&timer->it_lock, flags);



retry_delete:

	/*

	 * Even if the timer is not longer accessible from other tasks

	 * it still might be armed and queued in the underlying timer

	 * mechanism. Worse, that timer mechanism might run the expiry

	 * function concurrently.

	 */

	if (timer_delete_hook(timer) == TIMER_RETRY) {

		spin_unlock_irq(&timer->it_lock);

		/*

		 * Timer is expired concurrently, prevent livelocks

		 * and pointless spinning on RT.

		 *

		 * timer_wait_running() drops timer::it_lock, which opens

		 * the possibility for another task to delete the timer.

		 *

		 * That's not possible here because this is invoked from

		 * do_exit() only for the last thread of the thread group.

		 * So no other task can access and delete that timer.

		 */

		if (WARN_ON_ONCE(timer_wait_running(timer, &flags) != timer))

			return;



		goto retry_delete;

	}

	list_del(&timer->list);



	spin_unlock_irq(&timer->it_lock);

	spin_unlock_irqrestore(&timer->it_lock, flags);

	release_posix_timer(timer, IT_ID_SET);

}



/*

 * This is called by do_exit or de_thread, only when nobody else can

 * modify the signal->posix_timers list. Yet we need sighand->siglock

 * to prevent the race with /proc/pid/timers.

 * Invoked from do_exit() when the last thread of a thread group exits.

 * At that point no other task can access the timers of the dying

 * task anymore.

 */

void exit_itimers(struct task_struct *tsk)

{
@@ -1067,10 +1092,12 @@ void exit_itimers(struct task_struct *tsk) 
	if (list_empty(&tsk->signal->posix_timers))

		return;



	/* Protect against concurrent read via /proc/$PID/timers */

	spin_lock_irq(&tsk->sighand->siglock);

	list_replace_init(&tsk->signal->posix_timers, &timers);

	spin_unlock_irq(&tsk->sighand->siglock);



	/* The timers are not longer accessible via tsk::signal */

	while (!list_empty(&timers)) {

		tmr = list_first_entry(&timers, struct k_itimer, list);

		itimer_delete(tmr);