From eaa63fdda566447f631f15f94e3054719039e5c8 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Wed, 23 Dec 2020 14:42:22 +0100
Subject: [PATCH] Kernel: Don't assert on PT_PEEK with kernelspace address

We were casting the address to Userspace<T> without validating it first
which is no good and will trap an assertion soon after.

Let's catch this sooner with an ASSERT in the Userspace<T> constructor
and update the PT_PEEK and PT_POKE handlers to avoid it.

Fixes #4505.
---
 AK/Userspace.h    |  2 ++
 Kernel/Ptrace.cpp | 17 ++++++++---------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/AK/Userspace.h b/AK/Userspace.h
index eb6bb0c44aa..244fc5fb6a4 100644
--- a/AK/Userspace.h
+++ b/AK/Userspace.h
@@ -26,6 +26,7 @@
 
 #pragma once
 
+#include <AK/Assertions.h>
 #include <AK/StdLibExtras.h>
 #include <AK/Types.h>
 
@@ -59,6 +60,7 @@ public:
     Userspace(FlatPtr ptr)
         : m_ptr(ptr)
     {
+        ASSERT(m_ptr < 0xc0000000);
     }
 
     FlatPtr ptr() const { return m_ptr; }
diff --git a/Kernel/Ptrace.cpp b/Kernel/Ptrace.cpp
index 61b60286700..656e2006068 100644
--- a/Kernel/Ptrace.cpp
+++ b/Kernel/Ptrace.cpp
@@ -129,21 +129,20 @@ KResultOr<u32> handle_syscall(const Kernel::Syscall::SC_ptrace_params& params, P
         Kernel::Syscall::SC_ptrace_peek_params peek_params;
         if (!copy_from_user(&peek_params, reinterpret_cast<Kernel::Syscall::SC_ptrace_peek_params*>(params.addr)))
             return -EFAULT;
-
-        // read validation is done inside 'peek_user_data'
-        auto result = peer->process().peek_user_data((FlatPtr)peek_params.address);
-        if (result.is_error())
+        if (!is_user_address(VirtualAddress { peek_params.address }))
             return -EFAULT;
+        auto result = peer->process().peek_user_data(Userspace<const u32*> { (FlatPtr)peek_params.address });
+        if (result.is_error())
+            return result.error();
         if (!copy_to_user(peek_params.out_data, &result.value()))
             return -EFAULT;
         break;
     }
 
-    case PT_POKE: {
-        Userspace<u32*> addr = reinterpret_cast<FlatPtr>(params.addr);
-        // write validation is done inside 'poke_user_data'
-        return peer->process().poke_user_data(addr, params.data);
-    }
+    case PT_POKE:
+        if (!is_user_address(VirtualAddress { params.addr }))
+            return -EFAULT;
+        return peer->process().poke_user_data(Userspace<u32*> { (FlatPtr)params.addr }, params.data);
 
     default:
         return -EINVAL;