From f08e91f67e24ea12a12d4b95627ec320860d690e Mon Sep 17 00:00:00 2001
From: Andrew Kaster <akaster@serenityos.org>
Date: Fri, 4 Mar 2022 18:05:24 -0700
Subject: [PATCH] Kernel: Don't check pledges or veil against code coverage
 data files

Coverage tools like LLVM's source-based coverage or GNU's --coverage
need to be able to write out coverage files from any binary, regardless
of its security posture. Not ignoring these pledges and veils means we
can't get our coverage data out without playing some serious tricks.

However this is pretty terrible for normal exeuction, so only skip these
checks when we explicitly configured userspace for coverage.
---
 Kernel/CMakeLists.txt                   |  4 ++++
 Kernel/FileSystem/VirtualFileSystem.cpp |  7 +++++++
 Kernel/Syscalls/open.cpp                | 27 +++++++++++++++++--------
 3 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt
index 002b977a2e..4b3ebf809e 100644
--- a/Kernel/CMakeLists.txt
+++ b/Kernel/CMakeLists.txt
@@ -527,6 +527,10 @@ if (ENABLE_KERNEL_COVERAGE_COLLECTION)
         ../Kernel/Syscall.cpp
     )
     set_source_files_properties(${KCOV_EXCLUDED_SOURCES} PROPERTIES COMPILE_FLAGS "-fno-sanitize-coverage=trace-pc")
+elseif (ENABLE_USERSPACE_COVERAGE_COLLECTION)
+    # Disable checking open() pledges and the veil for coverage data when building userspace with coverage
+    # so that binaries can write out coverage data even with pledges/veil
+    add_compile_definitions(SKIP_PATH_VALIDATION_FOR_COVERAGE_INSTRUMENTATION)
 endif()
 
 # Kernel Undefined Behavior Sanitizer (KUBSAN)
diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp
index 3b20e78859..bea549d0c6 100644
--- a/Kernel/FileSystem/VirtualFileSystem.cpp
+++ b/Kernel/FileSystem/VirtualFileSystem.cpp
@@ -771,6 +771,13 @@ ErrorOr<void> VirtualFileSystem::validate_path_against_process_veil(StringView p
     VERIFY(!path.contains("/../"sv) && !path.ends_with("/.."sv));
     VERIFY(!path.contains("/./"sv) && !path.ends_with("/."sv));
 
+#ifdef SKIP_PATH_VALIDATION_FOR_COVERAGE_INSTRUMENTATION
+    // Skip veil validation against profile data when coverage is enabled for userspace
+    // so that all processes can write out coverage data even with veils in place
+    if (KLexicalPath::basename(path).ends_with(".profraw"sv))
+        return {};
+#endif
+
     auto& unveiled_path = find_matching_unveiled_path(path);
     if (unveiled_path.permissions() == UnveilAccess::None) {
         dbgln("Rejecting path '{}' since it hasn't been unveiled.", path);
diff --git a/Kernel/Syscalls/open.cpp b/Kernel/Syscalls/open.cpp
index 941722ba85..c8e7797f66 100644
--- a/Kernel/Syscalls/open.cpp
+++ b/Kernel/Syscalls/open.cpp
@@ -7,6 +7,7 @@
 #include <Kernel/Debug.h>
 #include <Kernel/FileSystem/Custody.h>
 #include <Kernel/FileSystem/VirtualFileSystem.h>
+#include <Kernel/KLexicalPath.h>
 #include <Kernel/Net/LocalSocket.h>
 #include <Kernel/Process.h>
 
@@ -27,19 +28,29 @@ ErrorOr<FlatPtr> Process::sys$open(Userspace<Syscall::SC_open_params const*> use
     if (options & O_UNLINK_INTERNAL)
         return EINVAL;
 
-    if (options & O_WRONLY)
-        TRY(require_promise(Pledge::wpath));
-    else if (options & O_RDONLY)
-        TRY(require_promise(Pledge::rpath));
+    auto path = TRY(get_syscall_path_argument(params.path));
 
-    if (options & O_CREAT)
-        TRY(require_promise(Pledge::cpath));
+    // Disable checking open pledges when building userspace with coverage
+    // so that all processes can write out coverage data even with pledges
+    bool skip_pledge_verification = false;
+
+#ifdef SKIP_PATH_VALIDATION_FOR_COVERAGE_INSTRUMENTATION
+    if (KLexicalPath::basename(path->view()).ends_with(".profraw"sv))
+        skip_pledge_verification = true;
+#endif
+    if (!skip_pledge_verification) {
+        if (options & O_WRONLY)
+            TRY(require_promise(Pledge::wpath));
+        else if (options & O_RDONLY)
+            TRY(require_promise(Pledge::rpath));
+
+        if (options & O_CREAT)
+            TRY(require_promise(Pledge::cpath));
+    }
 
     // Ignore everything except permission bits.
     mode &= 0777;
 
-    auto path = TRY(get_syscall_path_argument(params.path));
-
     dbgln_if(IO_DEBUG, "sys$open(dirfd={}, path='{}', options={}, mode={})", dirfd, path->view(), options, mode);
 
     auto fd_allocation = TRY(allocate_fd());