summaryrefslogtreecommitdiff
path: root/Kernel/Time
diff options
context:
space:
mode:
authorAndreas Kling <kling@serenityos.org>2021-08-10 20:22:34 +0200
committerAndreas Kling <kling@serenityos.org>2021-08-10 21:51:05 +0200
commit11456ebc00529bdc63a2b6ea64654c3c295cd394 (patch)
tree8a6bd933e4f5b0290c2f4caaffe84d5584810bc9 /Kernel/Time
parent16979bec15afbd9b7b6c4a3dbd6474780e3bbbb5 (diff)
downloadserenity-11456ebc00529bdc63a2b6ea64654c3c295cd394.zip
Kernel: Close race window in timestamp update mechanism
As pointed out by 8infy, this mechanism is racy: WRITER: 1. ++update1; 2. write_data(); 3. ++update2; READER: 1. do { auto saved = update1; 2. read_data(); 3. } while (saved != update2); The following sequence can lead to a bogus/partial read: R1 R2 R3 W1 W2 W3 We close this race by incrementing the second update counter first: WRITER: 1. ++update2; 2. write_data(); 3. ++update1;
Diffstat (limited to 'Kernel/Time')
-rw-r--r--Kernel/Time/TimeManagement.cpp12
1 files changed, 6 insertions, 6 deletions
diff --git a/Kernel/Time/TimeManagement.cpp b/Kernel/Time/TimeManagement.cpp
index e6cdabe562..73d3739662 100644
--- a/Kernel/Time/TimeManagement.cpp
+++ b/Kernel/Time/TimeManagement.cpp
@@ -357,7 +357,7 @@ void TimeManagement::increment_time_since_boot_hpet()
auto delta_ns = HPET::the().update_time(seconds_since_boot, ticks_this_second, false);
// Now that we have a precise time, go update it as quickly as we can
- u32 update_iteration = m_update1.fetch_add(1, AK::MemoryOrder::memory_order_acquire);
+ u32 update_iteration = m_update2.fetch_add(1, AK::MemoryOrder::memory_order_acquire);
m_seconds_since_boot = seconds_since_boot;
m_ticks_this_second = ticks_this_second;
// TODO: Apply m_remaining_epoch_time_adjustment
@@ -365,7 +365,7 @@ void TimeManagement::increment_time_since_boot_hpet()
update_time_page();
- m_update2.store(update_iteration + 1, AK::MemoryOrder::memory_order_release);
+ m_update1.store(update_iteration + 1, AK::MemoryOrder::memory_order_release);
}
void TimeManagement::increment_time_since_boot()
@@ -378,7 +378,7 @@ void TimeManagement::increment_time_since_boot()
long NanosPerTick = 1'000'000'000 / m_time_keeper_timer->frequency();
time_t MaxSlewNanos = NanosPerTick / 100;
- u32 update_iteration = m_update1.fetch_add(1, AK::MemoryOrder::memory_order_acquire);
+ u32 update_iteration = m_update2.fetch_add(1, AK::MemoryOrder::memory_order_acquire);
// Clamp twice, to make sure intermediate fits into a long.
long slew_nanos = clamp(clamp(m_remaining_epoch_time_adjustment.tv_sec, (time_t)-1, (time_t)1) * 1'000'000'000 + m_remaining_epoch_time_adjustment.tv_nsec, -MaxSlewNanos, MaxSlewNanos);
@@ -397,7 +397,7 @@ void TimeManagement::increment_time_since_boot()
}
update_time_page();
- m_update2.store(update_iteration + 1, AK::MemoryOrder::memory_order_release);
+ m_update1.store(update_iteration + 1, AK::MemoryOrder::memory_order_release);
}
void TimeManagement::system_timer_tick(const RegisterState& regs)
@@ -430,9 +430,9 @@ bool TimeManagement::disable_profile_timer()
void TimeManagement::update_time_page()
{
auto* page = time_page();
- u32 update_iteration = AK::atomic_fetch_add(&page->update1, 1u, AK::MemoryOrder::memory_order_acquire);
+ u32 update_iteration = AK::atomic_fetch_add(&page->update2, 1u, AK::MemoryOrder::memory_order_acquire);
page->clocks[CLOCK_REALTIME] = m_epoch_time;
- AK::atomic_store(&page->update2, update_iteration + 1u, AK::MemoryOrder::memory_order_release);
+ AK::atomic_store(&page->update1, update_iteration + 1u, AK::MemoryOrder::memory_order_release);
}
TimePage* TimeManagement::time_page()