From c8edcf1d71b459ee8c95e232d19725e206dda1eb Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 13 Apr 2020 22:40:38 +0200
Subject: [PATCH] Kernel: Don't ignore validation result in ptrace(PT_PEEK)

Also mark all of the address validation functions [[nodiscard]] to turn
this kind of bug into a compile error in the future.
---
 Kernel/Process.h  | 20 ++++++++++----------
 Kernel/Ptrace.cpp |  3 ++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/Kernel/Process.h b/Kernel/Process.h
index 05f9cbe94b6..e4148148bb4 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -326,14 +326,14 @@ public:
     u32 m_ticks_in_user_for_dead_children { 0 };
     u32 m_ticks_in_kernel_for_dead_children { 0 };
 
-    bool validate_read_from_kernel(VirtualAddress, size_t) const;
+    [[nodiscard]] bool validate_read_from_kernel(VirtualAddress, size_t) const;
 
-    bool validate_read(const void*, size_t) const;
-    bool validate_write(void*, size_t) const;
+    [[nodiscard]] bool validate_read(const void*, size_t) const;
+    [[nodiscard]] bool validate_write(void*, size_t) const;
     template<typename T>
-    bool validate_read_typed(T* value, size_t count = 1) { return validate_read(value, sizeof(T) * count); }
+    [[nodiscard]] bool validate_read_typed(T* value, size_t count = 1) { return validate_read(value, sizeof(T) * count); }
     template<typename T>
-    bool validate_read_and_copy_typed(T* dest, const T* src)
+    [[nodiscard]] bool validate_read_and_copy_typed(T* dest, const T* src)
     {
         bool validated = validate_read_typed(src);
         if (validated) {
@@ -342,14 +342,14 @@ public:
         return validated;
     }
     template<typename T>
-    bool validate_write_typed(T* value, size_t count = 1) { return validate_write(value, sizeof(T) * count); }
+    [[nodiscard]] bool validate_write_typed(T* value, size_t count = 1) { return validate_write(value, sizeof(T) * count); }
     template<typename DataType, typename SizeType>
-    bool validate(const Syscall::MutableBufferArgument<DataType, SizeType>&);
+    [[nodiscard]] bool validate(const Syscall::MutableBufferArgument<DataType, SizeType>&);
     template<typename DataType, typename SizeType>
-    bool validate(const Syscall::ImmutableBufferArgument<DataType, SizeType>&);
+    [[nodiscard]] bool validate(const Syscall::ImmutableBufferArgument<DataType, SizeType>&);
 
-    String validate_and_copy_string_from_user(const char*, size_t) const;
-    String validate_and_copy_string_from_user(const Syscall::StringArgument&) const;
+    [[nodiscard]] String validate_and_copy_string_from_user(const char*, size_t) const;
+    [[nodiscard]] String validate_and_copy_string_from_user(const Syscall::StringArgument&) const;
 
     Custody& current_directory();
     Custody* executable() { return m_executable.ptr(); }
diff --git a/Kernel/Ptrace.cpp b/Kernel/Ptrace.cpp
index f6f5d946d57..ca91e66913c 100644
--- a/Kernel/Ptrace.cpp
+++ b/Kernel/Ptrace.cpp
@@ -113,7 +113,8 @@ KResultOr<u32> handle_syscall(const Kernel::Syscall::SC_ptrace_params& params, P
         auto result = peer->process().peek_user_data(peek_params.address);
         if (result.is_error())
             return -EFAULT;
-        peer->process().validate_write(peek_params.out_data, sizeof(u32));
+        if (!peer->process().validate_write(peek_params.out_data, sizeof(u32)))
+            return -EFAULT;
         copy_from_user(peek_params.out_data, &result.value());
         break;
     }