diff options
author | Andreas Kling <kling@serenityos.org> | 2021-08-10 20:22:34 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2021-08-10 21:51:05 +0200 |
commit | 11456ebc00529bdc63a2b6ea64654c3c295cd394 (patch) | |
tree | 8a6bd933e4f5b0290c2f4caaffe84d5584810bc9 /Kernel/Time | |
parent | 16979bec15afbd9b7b6c4a3dbd6474780e3bbbb5 (diff) | |
download | serenity-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.cpp | 12 |
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() |