Kernel+CrashDaemon: Use a ".partial" suffix for incomplete coredumps

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

@@ -28,11 +28,16 @@ ErrorOr<NonnullOwnPtr<KString>> SysFSCoredumpDirectory::value() const
         return KString::try_create(""sv);
     });
 }
-void SysFSCoredumpDirectory::set_value(NonnullOwnPtr<KString> new_value)
+
+ErrorOr<void> SysFSCoredumpDirectory::set_value(NonnullOwnPtr<KString> new_value)
 {
+    if (new_value->length() > 0 && new_value->bytes()[0] != '/')
+        return Error::from_errno(EINVAL);
+
     Coredump::directory_path().with([&](auto& coredump_directory_path) {
         coredump_directory_path = move(new_value);
     });
+    return {};
 }
 
 mode_t SysFSCoredumpDirectory::permissions() const

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

@@ -20,7 +20,7 @@ class SysFSCoredumpDirectory final : public SysFSSystemStringVariable {
 
 private:
     virtual ErrorOr<NonnullOwnPtr<KString>> value() const override;
-    virtual void set_value(NonnullOwnPtr<KString> new_value) override;
+    virtual ErrorOr<void> set_value(NonnullOwnPtr<KString> new_value) override;
 
     explicit SysFSCoredumpDirectory(SysFSDirectory const&);
 

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

@@ -28,7 +28,7 @@ ErrorOr<size_t> SysFSSystemStringVariable::write_bytes(off_t, size_t count, User
     // NOTE: If we are in a jail, don't let the current process to change the variable.
     if (Process::current().is_jailed())
         return Error::from_errno(EPERM);
-    set_value(move(new_value_without_possible_newlines));
+    TRY(set_value(move(new_value_without_possible_newlines)));
     return count;
 }
 

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

@@ -31,7 +31,7 @@ class SysFSSystemStringVariable : public SysFSGlobalInformation {
     {
     }
     virtual ErrorOr<NonnullOwnPtr<KString>> value() const = 0;
-    virtual void set_value(NonnullOwnPtr<KString> new_value) = 0;
+    virtual ErrorOr<void> set_value(NonnullOwnPtr<KString> new_value) = 0;
 
 private:
     // ^SysFSGlobalInformation

Kernel/Tasks/Process.cpp

@@ -831,10 +831,19 @@ ErrorOr<void> Process::dump_core()
         return {};
     }
     auto coredump_path = TRY(name().with([&](auto& process_name) {
-        return KString::formatted("{}/{}_{}_{}", coredump_directory_path->view(), process_name.representable_view(), pid().value(), kgettimeofday().seconds_since_epoch());
+        return KString::formatted("{}/{}_{}_{}.partial", coredump_directory_path->view(), process_name.representable_view(), pid().value(), kgettimeofday().seconds_since_epoch());
     }));
     auto coredump = TRY(Coredump::try_create(*this, coredump_path->view()));
-    return coredump->write();
+    TRY(coredump->write());
+
+    auto root_custody = vfs_root_context()->root_custody().with([](auto& custody) -> NonnullRefPtr<Custody> {
+        return custody;
+    });
+
+    auto new_path = TRY(KString::try_create(coredump_path->view().trim(".partial"sv)));
+    TRY(VirtualFileSystem::rename(vfs_root_context(), credentials(), *root_custody, coredump_path->view(), *root_custody, new_path->view()));
+
+    return {};
 }
 
 ErrorOr<void> Process::dump_perfcore()

Tests/Kernel/TestProcFSWrite.cpp

@@ -41,3 +41,16 @@ TEST_CASE(root_writes_to_procfs)
         FAIL("Wrote successfully?!");
     }
 }
+
+TEST_CASE(set_coredump_path)
+{
+    auto fd = open("/sys/kernel/conf/coredump_directory", O_RDWR);
+    if (fd < 0) {
+        perror("open");
+        FAIL("open failed?! See debugout");
+        return;
+    }
+    static constexpr auto path = "relative/path"sv;
+    write(fd, path.characters_without_null_termination(), path.length());
+    EXPECT_EQ(errno, EINVAL);
+}

Userland/Services/CrashDaemon/main.cpp

@@ -4,32 +4,11 @@
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include <AK/LexicalPath.h>
-#include <Kernel/API/InodeWatcherEvent.h>
 #include <LibCore/FileWatcher.h>
 #include <LibCore/MappedFile.h>
 #include <LibCore/Process.h>
 #include <LibCore/System.h>
 #include <LibMain/Main.h>
-#include <spawn.h>
-#include <sys/stat.h>
-#include <time.h>
-#include <unistd.h>
-
-static void wait_until_coredump_is_ready(ByteString const& coredump_path)
-{
-    while (true) {
-        struct stat statbuf;
-        if (stat(coredump_path.characters(), &statbuf) < 0) {
-            perror("stat");
-            VERIFY_NOT_REACHED();
-        }
-        if (statbuf.st_mode & 0400) // Check if readable
-            break;
-
-        usleep(10000); // sleep for 10ms
-    }
-}
 
 static void launch_crash_reporter(ByteString const& coredump_path, bool unlink_on_exit)
 {
@@ -49,13 +28,14 @@ ErrorOr<int> serenity_main(Main::Arguments)
     TRY(watcher.add_watch("/tmp/coredump", Core::FileWatcherEvent::Type::ChildCreated));
 
     while (true) {
-        auto event = watcher.wait_for_event();
-        VERIFY(event.has_value());
-        if (event.value().type != Core::FileWatcherEvent::Type::ChildCreated)
+        auto event = watcher.wait_for_event().value();
+        if (event.type != Core::FileWatcherEvent::Type::ChildCreated)
             continue;
-        auto& coredump_path = event.value().event_path;
+        auto& coredump_path = event.event_path;
+        if (coredump_path.ends_with(".partial"sv))
+            continue;
+
         dbgln("New coredump file: {}", coredump_path);
-        wait_until_coredump_is_ready(coredump_path);
 
         auto file_or_error = Core::MappedFile::map(coredump_path);
         if (file_or_error.is_error()) {