[PATCH v2] rseq: Prevent self-preemption when granting a time slice extension

[Yao Kai]

__rseq_grant_slice_extension() makes the grant visible to user space and
then reads the clock to compute the expiry time before it clears the
pending reschedule request. This runs in the exit to user mode loop with
interrupts enabled.

Clocksource read functions are free to disable and enable preemption.
kvm_clock_read(), xen_clocksource_read() and the arm64 architected timer
counter accessors, which are used when a counter erratum workaround is
active, do so. With full or lazy preemption, when the request which
triggered the grant is a non-lazy TIF_NEED_RESCHED, e.g. a lazy request
upgraded by the scheduler tick, then preempt_enable_notrace() in the clock
read function schedules right away, e.g. in a KVM guest:

  __schedule
  preempt_schedule_notrace
  preempt_schedule_notrace_thunk
  kvm_clock_get_cycles
  ktime_get_mono_fast_ns
  irqentry_exit
  asm_sysvec_apic_timer_interrupt

The task is preempted with the grant already handed out to user space,
i.e. in the middle of the critical section which the extension is supposed
to protect. The stale grant is cleaned up when the task goes back to user
space, so the damage is limited to the extension silently not working for
such preemptions on systems which use one of these clocksources.

Unlike a preemption by an interrupt which hits after the grant has been
handed out, this does not depend on timing. The request which the grant
is made for is still pending when the clock is read, so every grant for a
non-lazy request is voided. With full preemption, where all requests are
non-lazy, the extension does not work at all on such systems.

Read the clock in the interrupt disabled section after the reschedule
request has been cleared.

The window between handing out the grant and disabling interrupts, in
which an interrupt can still preempt the task via
irqentry_exit_cond_resched(), is not affected by this change. That's a
race against a handful of instructions, which existed before.

Fixes: dfb630f548a7 ("rseq: Implement rseq_grant_slice_extension()")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yao Kai <yaokai34@xxxxxxxxxx>
---
 include/linux/rseq_entry.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/linux/rseq_entry.h b/include/linux/rseq_entry.h
index 31ce349ed42c..3e6a339175f8 100644
--- a/include/linux/rseq_entry.h
+++ b/include/linux/rseq_entry.h
@@ -174,8 +174,6 @@ static __always_inline bool __rseq_grant_slice_extension(bool work_pending)
 	rseq_stat_inc(rseq_stats.s_granted);
 
 	curr->rseq.slice.state.granted = true;
-	/* Store expiry time for arming the timer on the way out */
-	curr->rseq.slice.expires = data_race(rseq_slice_ext_nsecs) + ktime_get_mono_fast_ns();
 	/*
 	 * This is racy against a remote CPU setting TIF_NEED_RESCHED in
 	 * several ways:
@@ -222,6 +220,18 @@ static __always_inline bool __rseq_grant_slice_extension(bool work_pending)
 	scoped_guard(irq) {
 		clear_tsk_need_resched(curr);
 		clear_preempt_need_resched();
+		/*
+		 * Store expiry time for arming the timer on the way out.
+		 *
+		 * Read the clock only after the reschedule request has been
+		 * cleared and with interrupts disabled. Clocksource read
+		 * functions, e.g. kvm_clock_read(), may disable and enable
+		 * preemption, which would otherwise schedule right here on a
+		 * pending request and void the grant which was just handed
+		 * out to user space.
+		 */
+		curr->rseq.slice.expires = data_race(rseq_slice_ext_nsecs) +
+					   ktime_get_mono_fast_ns();
 	}
 	return true;
 
-- 
2.43.0