Repository: SerenityOS/serenity
File: Kernel/Tasks/Process.cpp (C++)

Code before:
    VERIFY(is_dumpable());
    VERIFY(should_generate_coredump());
    dbgln("Generating coredump for pid: {}", pid().value());
    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(name().with([&](auto& process_name) {
        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()));
    TRY(coredump->write());

    auto output_directory = KLexicalPath::dirname(coredump_path->view());
    auto vfs_root_context_root_custody = vfs_root_context()->root_custody().with([](auto& custody) -> NonnullRefPtr<Custody> {
        return custody;
    });
    auto dump_directory = TRY(VirtualFileSystem::open_directory(vfs_root_context(), credentials(), output_directory, *vfs_root_context_root_custody));

    auto new_path = TRY(KString::try_create(coredump_path->view().trim(".partial"sv)));
    TRY(VirtualFileSystem::rename(vfs_root_context(), credentials(), dump_directory, coredump_path->view(), dump_directory, new_path->view()));

    return {};
}

ErrorOr<void> Process::dump_perfcore()
{
    VERIFY(is_dumpable());
    VERIFY(m_perf_event_buffer);
    dbgln("Generating perfcore for pid: {}", pid().value());

    // Try to generate a filename which isn't already used.
    auto base_filename = TRY(name().with([&](auto& process_name) {
        return KString::formatted("{}_{}", process_name.representable_view(), pid().value());
    }));
    auto perfcore_filename = TRY(KString::formatted("{}.profile", base_filename));
    RefPtr<OpenFileDescription> description;
    auto credentials = this->credentials();
    for (size_t attempt = 1; attempt <= 10; ++attempt) {
        auto description_or_error = VirtualFileSystem::open(*this, vfs_root_context(), credentials, perfcore_filename->view(), O_CREAT | O_EXCL, 0400, current_directory(), UidAndGid { 0, 0 });
        if (!description_or_error.is_error()) {
            description = description_or_error.release_value();
            break;
        }
        perfcore_filename = TRY(KString::formatted("{}.{}.profile", base_filename, attempt));
    }

Review comment (suggestion):
The two `CustodyBase`s `rename` expects are mapped to `renameat()`, so if the given paths are absolute, the dirfd/`CustodyBase` is ignored (see also https://github.com/LucasChollet/serenity/blob/824d5bf04ce331681895caf4eedbf520a21757e2/Kernel/FileSystem/VirtualFileSystem.cpp#L1086).
`CustodyBase::resolve()` returns the root custody if the given path is absolute, so I think you should be able to do that here as well. The coredump path should always be absolute, though we currently don't seem to check that when userspace writes the SysFS variable.

(side note: While looking at the `CustodyBase` class, I noticed that it's maybe a bit counterintuitive for it to store the path in the class itself rather than taking it as an argument for `resolve()`)

Code after:
    VERIFY(is_dumpable());
    VERIFY(should_generate_coredump());
    dbgln("Generating coredump for pid: {}", pid().value());
    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(name().with([&](auto& process_name) {
        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()));
    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()
{
    VERIFY(is_dumpable());
    VERIFY(m_perf_event_buffer);
    dbgln("Generating perfcore for pid: {}", pid().value());

    // Try to generate a filename which isn't already used.
    auto base_filename = TRY(name().with([&](auto& process_name) {
        return KString::formatted("{}_{}", process_name.representable_view(), pid().value());
    }));
    auto perfcore_filename = TRY(KString::formatted("{}.profile", base_filename));
    RefPtr<OpenFileDescription> description;
    auto credentials = this->credentials();
    for (size_t attempt = 1; attempt <= 10; ++attempt) {
        auto description_or_error = VirtualFileSystem::open(*this, vfs_root_context(), credentials, perfcore_filename->view(), O_CREAT | O_EXCL, 0400, current_directory(), UidAndGid { 0, 0 });
        if (!description_or_error.is_error()) {
            description = description_or_error.release_value();
            break;
        }
        perfcore_filename = TRY(KString::formatted("{}.{}.profile", base_filename, attempt));
    }
    if (!description) {
        dbgln("Failed to generate perfcore for pid {}: Could not generate filename for the perfcore file.", pid().value());