Jelajahi Sumber

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.
Andrew Kaster 3 tahun lalu
induk
melakukan
f08e91f67e

+ 4 - 0
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)

+ 7 - 0
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);

+ 19 - 8
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());