From 11306d7121f414695c46ef9d6a46125c2cc5db9b Mon Sep 17 00:00:00 2001
From: Brian Gianforcaro <bgianf@serenityos.org>
Date: Wed, 5 May 2021 16:51:06 +0000
Subject: [PATCH] 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.
---
 Kernel/Syscalls/alarm.cpp      | 2 +-
 Kernel/Syscalls/clock.cpp      | 8 ++++----
 Kernel/ThreadBlockers.cpp      | 4 ++--
 Kernel/Time/TimeManagement.cpp | 5 +++--
 Kernel/Time/TimeManagement.h   | 2 +-
 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))));
 }