[PATCH] um: time-travel: fix time going backwards

From: Vincent Whitchurch
Date: Fri Oct 20 2023 - 10:47:51 EST


In basic time travel mode, I sometimes see "time goes backwards" panics
like the one below:

Kernel panic: time-travel: time goes backwards 161689340000492 -> 161689339869814
Call Trace:
panic+0x1a1/0x3d7
time_travel_update_time.cold+0xe9/0x133
timer_read+0xc1/0x100
ktime_get+0x10c/0x200
copy_process+0x1899/0x2230
kernel_clone+0x57/0x7a0
kernel_thread+0x4a/0x50
kthreadd+0x116/0x190

The problem is a race between time_travel_handle_real_alarm() and
timer_read(). time_travel_handle_real_alarm() changes the time after
time_read() reads the current time but before time_travel_update_time()
has had a chance to add the end event.

Fix this by doing the time read and event add atomically with respect to
time_travel_handle_real_alarm().

Signed-off-by: Vincent Whitchurch <vincent.whitchurch@xxxxxxxx>
---
arch/um/kernel/time.c | 48 ++++++++++++++++++++++++++++++++++--------------
1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/arch/um/kernel/time.c b/arch/um/kernel/time.c
index fddd1dec27e6..38a94dd41b8f 100644
--- a/arch/um/kernel/time.c
+++ b/arch/um/kernel/time.c
@@ -392,17 +392,11 @@ bool time_travel_del_event(struct time_travel_event *e)
return true;
}

-static void time_travel_update_time(unsigned long long next, bool idle)
+static void time_travel_update_to_event(struct time_travel_event *ne, bool idle)
{
- struct time_travel_event ne = {
- .onstack = true,
- };
struct time_travel_event *e;
bool finished = idle;

- /* add it without a handler - we deal with that specifically below */
- __time_travel_add_event(&ne, next);
-
do {
e = time_travel_first_event();

@@ -414,7 +408,7 @@ static void time_travel_update_time(unsigned long long next, bool idle)
BUG_ON(!time_travel_del_event(e));
BUG_ON(time_travel_time != e->time);

- if (e == &ne) {
+ if (e == ne) {
finished = true;
} else {
if (e->onstack)
@@ -427,14 +421,38 @@ static void time_travel_update_time(unsigned long long next, bool idle)
e = time_travel_first_event();
if (e)
time_travel_ext_update_request(e->time);
- } while (ne.pending && !finished);
+ } while (ne->pending && !finished);
+
+ time_travel_del_event(ne);
+}

- time_travel_del_event(&ne);
+static void time_travel_update_time(unsigned long long next, bool idle)
+{
+ struct time_travel_event ne = {
+ .onstack = true,
+ };
+
+ __time_travel_add_event(&ne, next);
+ time_travel_update_to_event(&ne, idle);
+}
+
+static void time_travel_forward_time(unsigned long nsec, bool idle)
+{
+ struct time_travel_event ne = {
+ .onstack = true,
+ };
+ unsigned long flags;
+
+ local_irq_save(flags);
+ __time_travel_add_event(&ne, time_travel_time + nsec);
+ local_irq_restore(flags);
+
+ time_travel_update_to_event(&ne, idle);
}

void time_travel_ndelay(unsigned long nsec)
{
- time_travel_update_time(time_travel_time + nsec, false);
+ time_travel_forward_time(nsec, false);
}
EXPORT_SYMBOL(time_travel_ndelay);

@@ -572,6 +590,10 @@ static inline void time_travel_update_time(unsigned long long ns, bool retearly)
{
}

+static void time_travel_forward_time(unsigned long nsec, bool idle)
+{
+}
+
static inline void time_travel_handle_real_alarm(void)
{
}
@@ -720,9 +742,7 @@ static u64 timer_read(struct clocksource *cs)
*/
if (!irqs_disabled() && !in_interrupt() && !in_softirq() &&
!time_travel_ext_waiting)
- time_travel_update_time(time_travel_time +
- TIMER_MULTIPLIER,
- false);
+ time_travel_forward_time(TIMER_MULTIPLIER, false);
return time_travel_time / TIMER_MULTIPLIER;
}


---
base-commit: 58720809f52779dc0f08e53e54b014209d13eebb
change-id: 20231020-uml-time-backwards-552c81aedd23

Best regards,
--
Vincent Whitchurch <vincent.whitchurch@xxxxxxxx>