diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index b61ba8e4e42..8ecf3b3a9e8 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -707,7 +707,11 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -static-pie") # kernel code that has been run to user space. It's rather slow and likely not # secure to run in production builds. Useful for coverage guided fuzzing. if (ENABLE_KERNEL_COVERAGE_COLLECTION) - add_definitions(-DENABLE_KERNEL_COVERAGE_COLLECTION) + add_compile_definitions(ENABLE_KERNEL_COVERAGE_COLLECTION) + if(ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG) + add_compile_definitions(ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG) + endif() + set(KERNEL_SOURCES ${KERNEL_SOURCES} Devices/KCOVDevice.cpp diff --git a/Kernel/SanCov.cpp b/Kernel/SanCov.cpp index 9346866158c..897d8ebe1b6 100644 --- a/Kernel/SanCov.cpp +++ b/Kernel/SanCov.cpp @@ -4,11 +4,31 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include +#include #include #include +#include extern bool g_in_early_boot; +#ifdef ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG +// Set kcov_emergency_off=true before making calls from __sanitizer_cov_trace_pc to coverage +// instrumented code, in order to prevent an infinite recursion. +// Any code reachable from the non-failure path in __sanitizer_cov_trace_pc must not be +// coverage instrumented. However, once a fatal error was detected, crash_and_burn will use +// a bunch of extra code to print useful debugging information. It would be wasteful not to +// instrument all of that code, so kcov_emergency_off=true can be used to bail out from +// recursive __sanitizer_cov_trace_pc calls while inside crash_and_burn. +bool kcov_emergency_off { false }; +static void crash_and_burn(Thread* thread) +{ + kcov_emergency_off = true; + thread->print_backtrace(); + PANIC("KCOV is b0rked."); + VERIFY_NOT_REACHED(); +} +#endif // Set ENABLE_KERNEL_COVERAGE_COLLECTION=ON via cmake, to inject this function on every program edge. // Note: This function is only used by fuzzing builds. When in use, it becomes an ultra hot code path. @@ -20,6 +40,28 @@ extern "C" void __sanitizer_cov_trace_pc(void) return; auto* thread = Processor::current_thread(); + +#ifdef ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG + if (kcov_emergency_off) [[unlikely]] + return; + + // Use are_interrupts_enabled() as a proxy to check we are not currently in an interrupt. + // current_in_irq() will only start returning true, once it incremented m_in_irq, which it + // doesn't do right away. This results in a short interval where we are in an interrupt, + // but the check will not tell us so. In that case, we would incorrectly identify the + // interrupt as __sanitizer_cov_trace_pc recursion here: + if (thread->m_kcov_recursion_hint && Processor::are_interrupts_enabled()) [[unlikely]] { + kcov_emergency_off = true; + dbgln("KCOV Error: __sanitizer_cov_trace_pc causes recursion. If possible, modify " + "__sanitizer_cov_trace_pc to not make the call which transitively caused the recursion. " + "Alternatively either mark the caller of the second __sanitizer_cov_trace_pc with " + "NO_SANITIZE_COVERAGE, or add that callers .cpp file to KCOV_EXCLUDED_SOURCES."); + crash_and_burn(thread); + VERIFY_NOT_REACHED(); + } + TemporaryChange kcov_recursion_hint { thread->m_kcov_recursion_hint, true }; +#endif + auto* kcov_instance = thread->process().kcov_instance(); if (kcov_instance == nullptr || !thread->m_kcov_enabled) [[likely]] return; // KCOV device hasn't been opened yet or thread is not traced diff --git a/Kernel/Tasks/Thread.h b/Kernel/Tasks/Thread.h index a1c47db88d4..8500ea650b8 100644 --- a/Kernel/Tasks/Thread.h +++ b/Kernel/Tasks/Thread.h @@ -1264,6 +1264,10 @@ public: #ifdef ENABLE_KERNEL_COVERAGE_COLLECTION // Used by __sanitizer_cov_trace_pc to identify traced threads. bool m_kcov_enabled { false }; +# ifdef ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG + // Used by __sanitizer_cov_trace_pc to detect an infinite recursion. + bool m_kcov_recursion_hint { false }; +# endif #endif }; diff --git a/Meta/CMake/all_the_debug_macros.cmake b/Meta/CMake/all_the_debug_macros.cmake index 76592e69499..8974c0a98ac 100644 --- a/Meta/CMake/all_the_debug_macros.cmake +++ b/Meta/CMake/all_the_debug_macros.cmake @@ -40,6 +40,8 @@ set(E1000_DEBUG ON) set(EDITOR_DEBUG ON) set(ELF_IMAGE_DEBUG ON) set(EMOJI_DEBUG ON) +set(ENABLE_KERNEL_COVERAGE_COLLECTION ON) +set(ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG ON) set(ESCAPE_SEQUENCE_DEBUG ON) set(ETHERNET_DEBUG ON) set(EVENT_DEBUG ON) diff --git a/Meta/CMake/serenity_options.cmake b/Meta/CMake/serenity_options.cmake index 3a8d78b504e..9cf39ed20c2 100644 --- a/Meta/CMake/serenity_options.cmake +++ b/Meta/CMake/serenity_options.cmake @@ -9,6 +9,7 @@ serenity_option(ENABLE_USB_IDS_DOWNLOAD ON CACHE BOOL "Enable download of the us serenity_option(ENABLE_PNP_IDS_DOWNLOAD ON CACHE BOOL "Enable download of the pnp.ids database at build time") serenity_option(ENABLE_KERNEL_ADDRESS_SANITIZER OFF CACHE BOOL "Enable kernel address sanitizer testing in gcc/clang") serenity_option(ENABLE_KERNEL_COVERAGE_COLLECTION OFF CACHE BOOL "Enable KCOV and kernel coverage instrumentation in gcc/clang") +serenity_option(ENABLE_KERNEL_COVERAGE_COLLECTION_DEBUG OFF CACHE BOOL "Enable KCOV and kernel coverage instrumentation debugging") serenity_option(ENABLE_KERNEL_LTO OFF CACHE BOOL "Build the kernel with link-time optimization") serenity_option(ENABLE_KERNEL_UNDEFINED_SANITIZER ON CACHE BOOL "Enable the Kernel Undefined Behavior Sanitizer (KUBSAN)") serenity_option(ENABLE_EXTRA_KERNEL_DEBUG_SYMBOLS OFF CACHE BOOL "Enable -Og and -ggdb3 options for Kernel code for easier debugging")