Revert "Kernel+SystemServer: Make KCOVDevice a character device"

This reverts commit 9dbec601b0.

For KCOV to be performant (or at least not even slower) we need to
mmap the PC buffer from both user and kernel space at the same time.
You can't mmap a character device, so this change didn't make sense.

Plus even if we did invent a new method to exfiltrate the coverage
information out of the kernel, it would be incompatible with existing
kernel fuzzers. That would be kind of annoying. 🙃
This commit is contained in:
Space Meyer 2024-04-07 19:12:31 +02:00 committed by Andrew Kaster
parent f870841bee
commit 106d4636a4
Notes: sideshowbarker 2024-07-17 04:10:16 +09:00
6 changed files with 13 additions and 9 deletions

View file

@ -28,7 +28,7 @@ UNMAP_AFTER_INIT NonnullLockRefPtr<KCOVDevice> KCOVDevice::must_create()
}
UNMAP_AFTER_INIT KCOVDevice::KCOVDevice()
: CharacterDevice(30, 0)
: BlockDevice(30, 0)
{
proc_instance = new HashMap<ProcessID, KCOVInstance*>();
thread_instance = new HashMap<ThreadID, KCOVInstance*>();

View file

@ -6,11 +6,11 @@
#pragma once
#include <Kernel/Devices/CharacterDevice.h>
#include <Kernel/Devices/BlockDevice.h>
#include <Kernel/Devices/KCOVInstance.h>
namespace Kernel {
class KCOVDevice final : public CharacterDevice {
class KCOVDevice final : public BlockDevice {
friend class DeviceManagement;
public:
@ -32,6 +32,7 @@ protected:
virtual bool can_read(OpenFileDescription const&, u64) const override final { return true; }
virtual bool can_write(OpenFileDescription const&, u64) const override final { return true; }
virtual void start_request(AsyncBlockDeviceRequest& request) override final { request.complete(AsyncDeviceRequest::Failure); }
virtual ErrorOr<size_t> read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override { return EINVAL; }
virtual ErrorOr<size_t> write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override { return EINVAL; }
virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override;

View file

@ -195,7 +195,6 @@ struct PluggableOnceCharacterDeviceNodeMatch {
static constexpr PluggableOnceCharacterDeviceNodeMatch s_simple_matchers[] = {
{ "/dev/beep"sv, 0666, 1, 10 },
{ "/dev/kcov"sv, 0666, 30, 0 },
};
static ErrorOr<void> create_pluggable_once_char_device_node(PluggableOnceCharacterDeviceNodeMatch const& match)
@ -251,10 +250,6 @@ ErrorOr<void> DeviceEventLoop::drain_events_from_devctl()
VERIFY(event.is_block_device == 1 || event.is_block_device == 0);
TRY(register_new_device(event.is_block_device ? DeviceNodeFamily::Type::BlockDevice : DeviceNodeFamily::Type::CharacterDevice, event.major_number, event.minor_number));
} else if (event.state == DeviceEvent::State::Removed) {
if (event.major_number == 30 && event.minor_number == 0 && !event.is_block_device) {
dbgln("DeviceMapper: unregistering device failed: kcov tried to de-register itself!?");
continue;
}
if (auto error_or_void = unregister_device(event.is_block_device ? DeviceNodeFamily::Type::BlockDevice : DeviceNodeFamily::Type::CharacterDevice, event.major_number, event.minor_number); error_or_void.is_error())
dbgln("DeviceMapper: unregistering device failed: {}", error_or_void.error());
} else {

View file

@ -11,3 +11,8 @@ set(SOURCES
serenity_bin(SystemServer)
target_link_libraries(SystemServer PRIVATE LibCore LibFileSystem LibMain)
# Required for conditionally creating hardcoded /dev/kcov entry
if (ENABLE_KERNEL_COVERAGE_COLLECTION)
add_compile_definitions(ENABLE_KERNEL_COVERAGE_COLLECTION)
endif()

View file

@ -139,6 +139,9 @@ static ErrorOr<void> prepare_bare_minimum_devtmpfs_directory_structure()
TRY(Core::System::create_char_device("/dev/console"sv, 0666, 5, 1));
TRY(Core::System::create_char_device("/dev/ptmx"sv, 0666, 5, 2));
TRY(Core::System::create_char_device("/dev/tty"sv, 0666, 5, 0));
#ifdef ENABLE_KERNEL_COVERAGE_COLLECTION
TRY(Core::System::create_block_device("/dev/kcov"sv, 0666, 30, 0));
#endif
umask(old_mask);
TRY(Core::System::symlink("/dev/random"sv, "/dev/urandom"sv));
TRY(Core::System::chmod("/dev/urandom"sv, 0666));

View file

@ -19,7 +19,7 @@ ErrorOr<int> serenity_main(Main::Arguments)
{
constexpr size_t num_entries = 1024 * 100;
int fd = TRY(Core::System::open("/dev/kcov0"sv, O_RDWR));
int fd = TRY(Core::System::open("/dev/kcov"sv, O_RDWR));
TRY(Core::System::ioctl(fd, KCOV_SETBUFSIZE, num_entries));
kcov_pc_t* cover = (kcov_pc_t*)TRY(Core::System::mmap(NULL, num_entries * KCOV_ENTRY_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0));
TRY(Core::System::ioctl(fd, KCOV_ENABLE));