Kernel+SystemServer: Don't hardcode coredump directory path

Instead, allow userspace to decide on the coredump directory path. By
default, SystemServer sets it to the /tmp/coredump directory, but users
can now change this by writing a new path to the sysfs node at
/sys/kernel/variables/coredump_directory, and also to read this node to
check where coredumps are currently generated at.

Kernel/CMakeLists.txt

@@ -196,8 +196,10 @@ set(KERNEL_SOURCES
     FileSystem/SysFS/Subsystems/Kernel/Network/UDP.cpp
     FileSystem/SysFS/Subsystems/Kernel/Variables/BooleanVariable.cpp
     FileSystem/SysFS/Subsystems/Kernel/Variables/CapsLockRemap.cpp
+    FileSystem/SysFS/Subsystems/Kernel/Variables/CoredumpDirectory.cpp
     FileSystem/SysFS/Subsystems/Kernel/Variables/Directory.cpp
     FileSystem/SysFS/Subsystems/Kernel/Variables/DumpKmallocStack.cpp
+    FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp
     FileSystem/SysFS/Subsystems/Kernel/Variables/UBSANDeadly.cpp
     FileSystem/TmpFS/FileSystem.cpp
     FileSystem/TmpFS/Inode.cpp

Kernel/Coredump.cpp

@@ -9,6 +9,7 @@
 
 #include <AK/ByteBuffer.h>
 #include <AK/JsonObjectSerializer.h>
+#include <AK/Singleton.h>
 #include <Kernel/Coredump.h>
 #include <Kernel/FileSystem/Custody.h>
 #include <Kernel/FileSystem/OpenFileDescription.h>
@@ -22,8 +23,15 @@
 
 #define INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS 0
 
+static Singleton<SpinlockProtected<OwnPtr<KString>>> s_coredump_directory_path;
+
 namespace Kernel {
 
+SpinlockProtected<OwnPtr<KString>>& Coredump::directory_path()
+{
+    return s_coredump_directory_path;
+}
+
 bool Coredump::FlatRegionData::looks_like_userspace_heap_region() const
 {
     return name().starts_with("LibJS:"sv) || name().starts_with("malloc:"sv);

Kernel/Coredump.h

@@ -11,6 +11,7 @@
 #include <AK/Vector.h>
 #include <Kernel/Forward.h>
 #include <Kernel/Library/NonnullLockRefPtr.h>
+#include <Kernel/Locking/SpinlockProtected.h>
 #include <Kernel/Memory/Region.h>
 
 namespace Kernel {
@@ -18,6 +19,7 @@ namespace Kernel {
 class Coredump {
 public:
     static ErrorOr<NonnullOwnPtr<Coredump>> try_create(NonnullLockRefPtr<Process>, StringView output_path);
+    static SpinlockProtected<OwnPtr<KString>>& directory_path();
 
     ~Coredump() = default;
     ErrorOr<void> write();

Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/CoredumpDirectory.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Kernel/Coredump.h>
+#include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/CoredumpDirectory.h>
+#include <Kernel/Sections.h>
+
+namespace Kernel {
+
+UNMAP_AFTER_INIT SysFSCoredumpDirectory::SysFSCoredumpDirectory(SysFSDirectory const& parent_directory)
+    : SysFSSystemStringVariable(parent_directory)
+{
+}
+
+UNMAP_AFTER_INIT NonnullLockRefPtr<SysFSCoredumpDirectory> SysFSCoredumpDirectory::must_create(SysFSDirectory const& parent_directory)
+{
+    return adopt_lock_ref_if_nonnull(new (nothrow) SysFSCoredumpDirectory(parent_directory)).release_nonnull();
+}
+
+ErrorOr<NonnullOwnPtr<KString>> SysFSCoredumpDirectory::value() const
+{
+    return Coredump::directory_path().with([&](auto& coredump_directory_path) -> ErrorOr<NonnullOwnPtr<KString>> {
+        if (coredump_directory_path)
+            return KString::try_create(coredump_directory_path->view());
+        return KString::try_create(""sv);
+    });
+}
+void SysFSCoredumpDirectory::set_value(NonnullOwnPtr<KString> new_value)
+{
+    Coredump::directory_path().with([&](auto& coredump_directory_path) {
+        coredump_directory_path = move(new_value);
+    });
+}
+
+mode_t SysFSCoredumpDirectory::permissions() const
+{
+    // NOTE: Let's not allow users to randomly change the coredump path, but only
+    // allow this for the root user.
+    return S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+}
+
+}

Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/CoredumpDirectory.h

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/Types.h>
+#include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.h>
+#include <Kernel/Library/LockRefPtr.h>
+#include <Kernel/UserOrKernelBuffer.h>
+
+namespace Kernel {
+
+class SysFSCoredumpDirectory final : public SysFSSystemStringVariable {
+public:
+    virtual StringView name() const override { return "coredump_directory"sv; }
+    static NonnullLockRefPtr<SysFSCoredumpDirectory> must_create(SysFSDirectory const&);
+
+private:
+    virtual ErrorOr<NonnullOwnPtr<KString>> value() const override;
+    virtual void set_value(NonnullOwnPtr<KString> new_value) override;
+
+    explicit SysFSCoredumpDirectory(SysFSDirectory const&);
+
+    virtual mode_t permissions() const override;
+};
+
+}

Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/Directory.cpp

@@ -8,6 +8,7 @@
 #include <AK/Try.h>
 #include <Kernel/FileSystem/SysFS/Component.h>
 #include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/CapsLockRemap.h>
+#include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/CoredumpDirectory.h>
 #include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/Directory.h>
 #include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/DumpKmallocStack.h>
 #include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/UBSANDeadly.h>
@@ -21,6 +22,7 @@ UNMAP_AFTER_INIT NonnullLockRefPtr<SysFSGlobalKernelVariablesDirectory> SysFSGlo
         list.append(SysFSCapsLockRemap::must_create(*global_variables_directory));
         list.append(SysFSDumpKmallocStacks::must_create(*global_variables_directory));
         list.append(SysFSUBSANDeadly::must_create(*global_variables_directory));
+        list.append(SysFSCoredumpDirectory::must_create(*global_variables_directory));
         return {};
     }));
     return global_variables_directory;

Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.cpp

@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.h>
+#include <Kernel/Process.h>
+#include <Kernel/Sections.h>
+
+namespace Kernel {
+
+ErrorOr<void> SysFSSystemStringVariable::try_generate(KBufferBuilder& builder)
+{
+    auto string_value = TRY(value());
+    return builder.appendff("{}\n", string_value->view());
+}
+
+ErrorOr<size_t> SysFSSystemStringVariable::write_bytes(off_t, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription*)
+{
+    MutexLocker locker(m_refresh_lock);
+    // Note: We do all of this code before taking the spinlock because then we disable
+    // interrupts so page faults will not work.
+    char* value = nullptr;
+    auto new_value = TRY(KString::try_create_uninitialized(count, value));
+    TRY(buffer.read(value, count));
+    auto new_value_without_possible_newlines = TRY(KString::try_create(new_value->view().trim("\n"sv)));
+    return Process::current().jail().with([&](auto& my_jail) -> ErrorOr<size_t> {
+        // Note: If we are in a jail, don't let the current process to change the variable.
+        if (my_jail)
+            return Error::from_errno(EPERM);
+        set_value(move(new_value_without_possible_newlines));
+        return count;
+    });
+}
+
+ErrorOr<void> SysFSSystemStringVariable::truncate(u64 size)
+{
+    if (size != 0)
+        return EPERM;
+    return {};
+}
+
+}

Kernel/FileSystem/SysFS/Subsystems/Kernel/Variables/StringVariable.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2022, Liav A. <liavalb@hotmail.co.il>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/AtomicRefCounted.h>
+#include <AK/Error.h>
+#include <AK/Function.h>
+#include <AK/OwnPtr.h>
+#include <AK/Types.h>
+#include <Kernel/FileSystem/File.h>
+#include <Kernel/FileSystem/FileSystem.h>
+#include <Kernel/FileSystem/OpenFileDescription.h>
+#include <Kernel/FileSystem/SysFS/Subsystems/Kernel/GlobalInformation.h>
+#include <Kernel/KBufferBuilder.h>
+#include <Kernel/KString.h>
+#include <Kernel/Library/LockRefPtr.h>
+#include <Kernel/Locking/Mutex.h>
+#include <Kernel/Time/TimeManagement.h>
+#include <Kernel/UserOrKernelBuffer.h>
+
+namespace Kernel {
+
+class SysFSSystemStringVariable : public SysFSGlobalInformation {
+protected:
+    explicit SysFSSystemStringVariable(SysFSDirectory const& parent_directory)
+        : SysFSGlobalInformation(parent_directory)
+    {
+    }
+    virtual ErrorOr<NonnullOwnPtr<KString>> value() const = 0;
+    virtual void set_value(NonnullOwnPtr<KString> new_value) = 0;
+
+private:
+    // ^SysFSGlobalInformation
+    virtual ErrorOr<void> try_generate(KBufferBuilder&) override final;
+
+    // ^SysFSExposedComponent
+    virtual ErrorOr<size_t> write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) override final;
+    virtual mode_t permissions() const override { return 0644; }
+    virtual ErrorOr<void> truncate(u64) override final;
+};
+
+}

Kernel/Process.cpp

@@ -696,7 +696,16 @@ ErrorOr<void> Process::dump_core()
     VERIFY(is_dumpable());
     VERIFY(should_generate_coredump());
     dbgln("Generating coredump for pid: {}", pid().value());
-    auto coredump_path = TRY(KString::formatted("/tmp/coredump/{}_{}_{}", name(), pid().value(), kgettimeofday().to_truncated_seconds()));
+    auto coredump_directory_path = TRY(Coredump::directory_path().with([&](auto& coredump_directory_path) -> ErrorOr<NonnullOwnPtr<KString>> {
+        if (coredump_directory_path)
+            return KString::try_create(coredump_directory_path->view());
+        return KString::try_create(""sv);
+    }));
+    if (coredump_directory_path->view() == ""sv) {
+        dbgln("Generating coredump for pid {} failed because coredump directory was not set.", pid().value());
+        return {};
+    }
+    auto coredump_path = TRY(KString::formatted("{}/{}_{}_{}", coredump_directory_path->view(), name(), pid().value(), kgettimeofday().to_truncated_seconds()));
     auto coredump = TRY(Coredump::try_create(*this, coredump_path->view()));
     return coredump->write();
 }

Userland/Services/SystemServer/main.cpp

@@ -469,6 +469,19 @@ static ErrorOr<void> create_tmp_coredump_directory()
     return {};
 }
 
+static ErrorOr<void> set_default_coredump_directory()
+{
+    dbgln("Setting /tmp/coredump as the coredump directory");
+    auto sysfs_coredump_directory_variable_fd = TRY(Core::System::open("/sys/kernel/variables/coredump_directory"sv, O_RDWR));
+    ScopeGuard close_on_exit([&] {
+        close(sysfs_coredump_directory_variable_fd);
+    });
+    auto tmp_coredump_directory_path = "/tmp/coredump"sv;
+    auto nwritten = TRY(Core::System::write(sysfs_coredump_directory_variable_fd, tmp_coredump_directory_path.bytes()));
+    VERIFY(static_cast<size_t>(nwritten) == tmp_coredump_directory_path.length());
+    return {};
+}
+
 static ErrorOr<void> create_tmp_semaphore_directory()
 {
     dbgln("Creating /tmp/semaphore directory");
@@ -494,6 +507,7 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     if (!user) {
         TRY(create_tmp_coredump_directory());
+        TRY(set_default_coredump_directory());
         TRY(create_tmp_semaphore_directory());
         TRY(determine_system_mode());
     }