diff options
author | Brian Gianforcaro <bgianf@serenityos.org> | 2021-05-05 16:51:06 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-05 18:51:06 +0200 |
commit | 11306d7121f414695c46ef9d6a46125c2cc5db9b (patch) | |
tree | 00990bbbf5e6ac0453a0493d43e0a6a44cfdf042 | |
parent | 64b4e3f34b16c6aee35fd399874a3dc1dbeecb87 (diff) | |
download | serenity-11306d7121f414695c46ef9d6a46125c2cc5db9b.zip |
Kernel: Modify TimeManagement::current_time(..) API so it can't fail. (#6869)
The fact that current_time can "fail" makes its use a bit awkward.
All callers in the Kernel are trusted besides syscalls, so assert
that they never get there, and make sure all current callers perform
validation of the clock_id with TimeManagement::is_valid_clock_id().
I have fuzzed this change locally for a bit to make sure I didn't
miss any obvious regression.
-rw-r--r-- | Kernel/Syscalls/alarm.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/clock.cpp | 8 | ||||
-rw-r--r-- | Kernel/ThreadBlockers.cpp | 4 | ||||
-rw-r--r-- | Kernel/Time/TimeManagement.cpp | 5 | ||||
-rw-r--r-- | Kernel/Time/TimeManagement.h | 2 | ||||
-rw-r--r-- | Kernel/TimerQueue.cpp | 6 |
6 files changed, 14 insertions, 13 deletions
diff --git a/Kernel/Syscalls/alarm.cpp b/Kernel/Syscalls/alarm.cpp index 144f70ce85..221df0d233 100644 --- a/Kernel/Syscalls/alarm.cpp +++ b/Kernel/Syscalls/alarm.cpp @@ -25,7 +25,7 @@ KResultOr<unsigned> Process::sys$alarm(unsigned seconds) } if (seconds > 0) { - auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE).value(); + auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE); deadline = deadline + Time::from_seconds(seconds); m_alarm_timer = TimerQueue::the().add_timer_without_id(CLOCK_REALTIME_COARSE, deadline, [this]() { [[maybe_unused]] auto rc = send_signal(SIGALRM, nullptr); diff --git a/Kernel/Syscalls/clock.cpp b/Kernel/Syscalls/clock.cpp index 15f83bdfcf..a1bdac9bd6 100644 --- a/Kernel/Syscalls/clock.cpp +++ b/Kernel/Syscalls/clock.cpp @@ -14,13 +14,13 @@ KResultOr<int> Process::sys$clock_gettime(clockid_t clock_id, Userspace<timespec { REQUIRE_PROMISE(stdio); - auto time = TimeManagement::the().current_time(clock_id); - if (time.is_error()) - return time.error(); + if (!TimeManagement::is_valid_clock_id(clock_id)) + return EINVAL; - auto ts = time.value().to_timespec(); + auto ts = TimeManagement::the().current_time(clock_id).to_timespec(); if (!copy_to_user(user_ts, &ts)) return EFAULT; + return 0; } diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp index c63122a453..98a9e71bd6 100644 --- a/Kernel/ThreadBlockers.cpp +++ b/Kernel/ThreadBlockers.cpp @@ -23,7 +23,7 @@ Thread::BlockTimeout::BlockTimeout(bool is_absolute, const Time* time, const Tim m_time = *time; m_should_block = true; } - m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id).value(); + m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id); if (!is_absolute) m_time += m_start_time; } @@ -326,7 +326,7 @@ void Thread::SleepBlocker::calculate_remaining() { if (!m_remaining) return; - auto time_now = TimeManagement::the().current_time(m_deadline.clock_id()).value(); + auto time_now = TimeManagement::the().current_time(m_deadline.clock_id()); if (time_now < m_deadline.absolute_time()) *m_remaining = m_deadline.absolute_time() - time_now; else diff --git a/Kernel/Time/TimeManagement.cpp b/Kernel/Time/TimeManagement.cpp index 3305eeb636..153b02bf21 100644 --- a/Kernel/Time/TimeManagement.cpp +++ b/Kernel/Time/TimeManagement.cpp @@ -44,7 +44,7 @@ bool TimeManagement::is_valid_clock_id(clockid_t clock_id) }; } -KResultOr<Time> TimeManagement::current_time(clockid_t clock_id) const +Time TimeManagement::current_time(clockid_t clock_id) const { switch (clock_id) { case CLOCK_MONOTONIC: @@ -58,7 +58,8 @@ KResultOr<Time> TimeManagement::current_time(clockid_t clock_id) const case CLOCK_REALTIME_COARSE: return epoch_time(TimePrecision::Coarse); default: - return KResult(EINVAL); + // Syscall entrypoint is missing a is_valid_clock_id(..) check? + VERIFY_NOT_REACHED(); } } diff --git a/Kernel/Time/TimeManagement.h b/Kernel/Time/TimeManagement.h index a178687a5e..0c48dd79c6 100644 --- a/Kernel/Time/TimeManagement.h +++ b/Kernel/Time/TimeManagement.h @@ -34,7 +34,7 @@ public: static TimeManagement& the(); static bool is_valid_clock_id(clockid_t); - KResultOr<Time> current_time(clockid_t) const; + Time current_time(clockid_t) const; Time monotonic_time(TimePrecision = TimePrecision::Coarse) const; Time monotonic_time_raw() const { diff --git a/Kernel/TimerQueue.cpp b/Kernel/TimerQueue.cpp index b7dd9efc45..727499c81e 100644 --- a/Kernel/TimerQueue.cpp +++ b/Kernel/TimerQueue.cpp @@ -44,7 +44,7 @@ Time Timer::now(bool is_firing) const break; } } - return TimeManagement::the().current_time(clock_id).value(); + return TimeManagement::the().current_time(clock_id); } TimerQueue& TimerQueue::the() @@ -59,7 +59,7 @@ UNMAP_AFTER_INIT TimerQueue::TimerQueue() RefPtr<Timer> TimerQueue::add_timer_without_id(clockid_t clock_id, const Time& deadline, Function<void()>&& callback) { - if (deadline <= TimeManagement::the().current_time(clock_id).value()) + if (deadline <= TimeManagement::the().current_time(clock_id)) return {}; // Because timer handlers can execute on any processor and there is @@ -117,7 +117,7 @@ void TimerQueue::add_timer_locked(NonnullRefPtr<Timer> timer) TimerId TimerQueue::add_timer(clockid_t clock_id, const Time& deadline, Function<void()>&& callback) { - auto expires = TimeManagement::the().current_time(clock_id).value(); + auto expires = TimeManagement::the().current_time(clock_id); expires = expires + deadline; return add_timer(adopt_ref(*new Timer(clock_id, expires, move(callback)))); } |