From e6929835d2438fff5f10da11537dd7d1eb225a80 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 6 Sep 2021 22:22:14 +0200
Subject: [PATCH] Kernel: Make copy_time_from_user() helpers use
 KResultOr<Time>

...and use TRY() for smooth error propagation everywhere.
---
 Kernel/Net/Socket.cpp      | 14 ++------------
 Kernel/StdLib.cpp          | 25 +++++++++++--------------
 Kernel/StdLib.h            |  6 +++---
 Kernel/Syscalls/clock.cpp  | 20 +++++++-------------
 Kernel/Syscalls/futex.cpp  |  6 ++----
 Kernel/Syscalls/select.cpp | 12 ++++--------
 6 files changed, 29 insertions(+), 54 deletions(-)

diff --git a/Kernel/Net/Socket.cpp b/Kernel/Net/Socket.cpp
index ffda8c4ccfb..24f1f535af3 100644
--- a/Kernel/Net/Socket.cpp
+++ b/Kernel/Net/Socket.cpp
@@ -90,22 +90,12 @@ KResult Socket::setsockopt(int level, int option, Userspace<const void*> user_va
     case SO_SNDTIMEO:
         if (user_value_size != sizeof(timeval))
             return EINVAL;
-        {
-            auto timeout = copy_time_from_user(static_ptr_cast<const timeval*>(user_value));
-            if (!timeout.has_value())
-                return EFAULT;
-            m_send_timeout = timeout.value();
-        }
+        m_send_timeout = TRY(copy_time_from_user(static_ptr_cast<timeval const*>(user_value)));
         return KSuccess;
     case SO_RCVTIMEO:
         if (user_value_size != sizeof(timeval))
             return EINVAL;
-        {
-            auto timeout = copy_time_from_user(static_ptr_cast<const timeval*>(user_value));
-            if (!timeout.has_value())
-                return EFAULT;
-            m_receive_timeout = timeout.value();
-        }
+        m_receive_timeout = TRY(copy_time_from_user(static_ptr_cast<timeval const*>(user_value)));
         return KSuccess;
     case SO_BINDTODEVICE: {
         if (user_value_size != IFNAMSIZ)
diff --git a/Kernel/StdLib.cpp b/Kernel/StdLib.cpp
index 5bb48ea2b0f..68f4baa142d 100644
--- a/Kernel/StdLib.cpp
+++ b/Kernel/StdLib.cpp
@@ -40,31 +40,28 @@ Kernel::KResultOr<NonnullOwnPtr<Kernel::KString>> try_copy_kstring_from_user(Use
     return new_string;
 }
 
-[[nodiscard]] Optional<Time> copy_time_from_user(const timespec* ts_user)
+KResultOr<Time> copy_time_from_user(timespec const* ts_user)
 {
-    timespec ts;
-    if (copy_from_user(&ts, ts_user, sizeof(timespec)).is_error()) {
-        return {};
-    }
+    timespec ts {};
+    TRY(copy_from_user(&ts, ts_user, sizeof(timespec)));
     return Time::from_timespec(ts);
 }
-[[nodiscard]] Optional<Time> copy_time_from_user(const timeval* tv_user)
+
+KResultOr<Time> copy_time_from_user(timeval const* tv_user)
 {
-    timeval tv;
-    if (copy_from_user(&tv, tv_user, sizeof(timeval)).is_error()) {
-        return {};
-    }
+    timeval tv {};
+    TRY(copy_from_user(&tv, tv_user, sizeof(timeval)));
     return Time::from_timeval(tv);
 }
 
 template<>
-[[nodiscard]] Optional<Time> copy_time_from_user<const timeval>(Userspace<const timeval*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
+KResultOr<Time> copy_time_from_user<const timeval>(Userspace<timeval const*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
 template<>
-[[nodiscard]] Optional<Time> copy_time_from_user<timeval>(Userspace<timeval*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
+KResultOr<Time> copy_time_from_user<timeval>(Userspace<timeval*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
 template<>
-[[nodiscard]] Optional<Time> copy_time_from_user<const timespec>(Userspace<const timespec*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
+KResultOr<Time> copy_time_from_user<const timespec>(Userspace<timespec const*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
 template<>
-[[nodiscard]] Optional<Time> copy_time_from_user<timespec>(Userspace<timespec*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
+KResultOr<Time> copy_time_from_user<timespec>(Userspace<timespec*> src) { return copy_time_from_user(src.unsafe_userspace_ptr()); }
 
 Optional<u32> user_atomic_fetch_add_relaxed(volatile u32* var, u32 val)
 {
diff --git a/Kernel/StdLib.h b/Kernel/StdLib.h
index 0b9c3f9158a..65fc93aece0 100644
--- a/Kernel/StdLib.h
+++ b/Kernel/StdLib.h
@@ -19,10 +19,10 @@ struct StringArgument;
 }
 
 [[nodiscard]] Kernel::KResultOr<NonnullOwnPtr<Kernel::KString>> try_copy_kstring_from_user(Userspace<const char*>, size_t);
-[[nodiscard]] Optional<Time> copy_time_from_user(const timespec*);
-[[nodiscard]] Optional<Time> copy_time_from_user(const timeval*);
+KResultOr<Time> copy_time_from_user(timespec const*);
+KResultOr<Time> copy_time_from_user(timeval const*);
 template<typename T>
-[[nodiscard]] Optional<Time> copy_time_from_user(Userspace<T*> src);
+KResultOr<Time> copy_time_from_user(Userspace<T*>);
 
 [[nodiscard]] Optional<u32> user_atomic_fetch_add_relaxed(volatile u32* var, u32 val);
 [[nodiscard]] Optional<u32> user_atomic_exchange_relaxed(volatile u32* var, u32 val);
diff --git a/Kernel/Syscalls/clock.cpp b/Kernel/Syscalls/clock.cpp
index f5ad7d03c3a..a6dae062c26 100644
--- a/Kernel/Syscalls/clock.cpp
+++ b/Kernel/Syscalls/clock.cpp
@@ -42,13 +42,11 @@ KResultOr<FlatPtr> Process::sys$clock_settime(clockid_t clock_id, Userspace<cons
     if (!is_superuser())
         return EPERM;
 
-    auto ts = copy_time_from_user(user_ts);
-    if (!ts.has_value())
-        return EFAULT;
+    auto time = TRY(copy_time_from_user(user_ts));
 
     switch (clock_id) {
     case CLOCK_REALTIME:
-        TimeManagement::the().set_epoch_time(ts.value());
+        TimeManagement::the().set_epoch_time(time);
         break;
     default:
         return EINVAL;
@@ -62,9 +60,7 @@ KResultOr<FlatPtr> Process::sys$clock_nanosleep(Userspace<const Syscall::SC_cloc
     REQUIRE_PROMISE(stdio);
     auto params = TRY(copy_typed_from_user(user_params));
 
-    Optional<Time> requested_sleep = copy_time_from_user(params.requested_sleep);
-    if (!requested_sleep.has_value())
-        return EFAULT;
+    auto requested_sleep = TRY(copy_time_from_user(params.requested_sleep));
 
     bool is_absolute;
     switch (params.flags) {
@@ -83,10 +79,10 @@ KResultOr<FlatPtr> Process::sys$clock_nanosleep(Userspace<const Syscall::SC_cloc
 
     bool was_interrupted;
     if (is_absolute) {
-        was_interrupted = Thread::current()->sleep_until(params.clock_id, requested_sleep.value()).was_interrupted();
+        was_interrupted = Thread::current()->sleep_until(params.clock_id, requested_sleep).was_interrupted();
     } else {
         Time remaining_sleep;
-        was_interrupted = Thread::current()->sleep(params.clock_id, requested_sleep.value(), &remaining_sleep).was_interrupted();
+        was_interrupted = Thread::current()->sleep(params.clock_id, requested_sleep, &remaining_sleep).was_interrupted();
         timespec remaining_sleep_ts = remaining_sleep.to_timespec();
         if (was_interrupted && params.remaining_sleep) {
             TRY(copy_to_user(params.remaining_sleep, &remaining_sleep_ts));
@@ -111,12 +107,10 @@ KResultOr<FlatPtr> Process::sys$adjtime(Userspace<const timeval*> user_delta, Us
         REQUIRE_PROMISE(settime);
         if (!is_superuser())
             return EPERM;
-        auto delta = copy_time_from_user(user_delta);
-        if (!delta.has_value())
-            return EFAULT;
+        auto delta = TRY(copy_time_from_user(user_delta));
 
         // FIXME: Should use AK::Time internally
-        TimeManagement::the().set_remaining_epoch_time_adjustment(delta->to_timespec());
+        TimeManagement::the().set_remaining_epoch_time_adjustment(delta.to_timespec());
     }
 
     return 0;
diff --git a/Kernel/Syscalls/futex.cpp b/Kernel/Syscalls/futex.cpp
index 308c284b456..2080c8d63f5 100644
--- a/Kernel/Syscalls/futex.cpp
+++ b/Kernel/Syscalls/futex.cpp
@@ -41,12 +41,10 @@ KResultOr<FlatPtr> Process::sys$futex(Userspace<const Syscall::SC_futex_params*>
     case FUTEX_REQUEUE:
     case FUTEX_CMP_REQUEUE: {
         if (params.timeout) {
-            auto timeout_time = copy_time_from_user(params.timeout);
-            if (!timeout_time.has_value())
-                return EFAULT;
+            auto timeout_time = TRY(copy_time_from_user(params.timeout));
             bool is_absolute = cmd != FUTEX_WAIT;
             clockid_t clock_id = use_realtime_clock ? CLOCK_REALTIME_COARSE : CLOCK_MONOTONIC_COARSE;
-            timeout = Thread::BlockTimeout(is_absolute, &timeout_time.value(), nullptr, clock_id);
+            timeout = Thread::BlockTimeout(is_absolute, &timeout_time, nullptr, clock_id);
         }
         if (cmd == FUTEX_WAIT_BITSET && params.val3 == FUTEX_BITSET_MATCH_ANY)
             cmd = FUTEX_WAIT;
diff --git a/Kernel/Syscalls/select.cpp b/Kernel/Syscalls/select.cpp
index 9ece4b8463e..4f324f580df 100644
--- a/Kernel/Syscalls/select.cpp
+++ b/Kernel/Syscalls/select.cpp
@@ -26,10 +26,8 @@ KResultOr<FlatPtr> Process::sys$select(Userspace<const Syscall::SC_select_params
 
     Thread::BlockTimeout timeout;
     if (params.timeout) {
-        Optional<Time> timeout_time = copy_time_from_user(params.timeout);
-        if (!timeout_time.has_value())
-            return EFAULT;
-        timeout = Thread::BlockTimeout(false, &timeout_time.value());
+        auto timeout_time = TRY(copy_time_from_user(params.timeout));
+        timeout = Thread::BlockTimeout(false, &timeout_time);
     }
 
     auto current_thread = Thread::current();
@@ -134,10 +132,8 @@ KResultOr<FlatPtr> Process::sys$poll(Userspace<const Syscall::SC_poll_params*> u
 
     Thread::BlockTimeout timeout;
     if (params.timeout) {
-        auto timeout_time = copy_time_from_user(params.timeout);
-        if (!timeout_time.has_value())
-            return EFAULT;
-        timeout = Thread::BlockTimeout(false, &timeout_time.value());
+        auto timeout_time = TRY(copy_time_from_user(params.timeout));
+        timeout = Thread::BlockTimeout(false, &timeout_time);
     }
 
     sigset_t sigmask = {};