Merge pull request #5018 from SandstromPL/fix/relinker-replace-outputs

fix(relinker): replace output files instead of rewriting them in place

core/relinker/io/src/FileWriter.cpp

@@ -1,28 +1,110 @@
 #include <io/FileWriter.hpp>
 #include <io/NativePath.hpp>
 #include <domain/Types.hpp>
+#include <filesystem>
 #include <fstream>
+#include <system_error>
+#ifndef _WIN32
+#include <cerrno>
+#include <random>
+#include <string_view>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#endif
 
 namespace Io {
 
-void FileWriter::Write(const std::string& path, const std::vector<std::uint8_t>& data) {
-    std::ofstream f(NativePath(path), std::ios::binary);
+namespace {
+
+bool writeFile(const std::filesystem::path& file, const std::string& path, const std::ios::openmode mode, const char* data, const std::size_t size) {
+    std::ofstream f(file, mode);
     if (!f)
-        throw Domain::RelinkerException("Cannot open output file: " + path);
-    f.write(reinterpret_cast<const char*>(data.data()), static_cast<std::streamsize>(data.size()));
+        return false;
+    f.write(data, static_cast<std::streamsize>(size));
     f.close();
     if (!f)
         throw Domain::RelinkerException("Failed to write file: " + path);
+    return true;
+}
+
+#ifndef _WIN32
+int createTemporary(const std::filesystem::path& target, const mode_t mode, std::filesystem::path& temporary) {
+    constexpr std::string_view characters = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+    std::random_device device;
+    for (int attempt = 0; attempt < 100; ++attempt) {
+        std::string suffix = ".";
+        for (int index = 0; index < 6; ++index)
+            suffix += characters[device() % characters.size()];
+        temporary = target;
+        temporary += suffix;
+        const int descriptor = open(temporary.c_str(), O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode);
+        if (descriptor >= 0 || errno != EEXIST)
+            return descriptor;
+    }
+    return -1;
+}
+
+bool writeAll(const int descriptor, const char* data, std::size_t size) {
+    while (size != 0) {
+        const auto written = write(descriptor, data, size);
+        if (written < 0 && errno == EINTR)
+            continue;
+        if (written <= 0)
+            return false;
+        data += written;
+        size -= static_cast<std::size_t>(written);
+    }
+    return true;
+}
+
+bool replaceFile(const std::filesystem::path& target, const std::string& path, const char* data, const std::size_t size) {
+    std::error_code error;
+    const auto previous = std::filesystem::symlink_status(target, error);
+    const bool regular = std::filesystem::is_regular_file(previous);
+    if (!regular && previous.type() != std::filesystem::file_type::not_found)
+        return false;
+    const auto mode = regular ? static_cast<mode_t>(previous.permissions() & std::filesystem::perms::all) : static_cast<mode_t>(0666);
+    std::filesystem::path temporary;
+    const int descriptor = createTemporary(target, mode, temporary);
+    if (descriptor < 0)
+        return false;
+    if (regular && fchmod(descriptor, mode) != 0) {
+        close(descriptor);
+        std::filesystem::remove(temporary, error);
+        return false;
+    }
+    const bool written = writeAll(descriptor, data, size);
+    if (close(descriptor) != 0 || !written) {
+        std::filesystem::remove(temporary, error);
+        throw Domain::RelinkerException("Failed to write file: " + path);
+    }
+    std::filesystem::rename(temporary, target, error);
+    if (!error)
+        return true;
+    std::filesystem::remove(temporary, error);
+    return false;
+}
+#endif
+
+void writeOutput(const std::string& path, const std::ios::openmode mode, const char* data, const std::size_t size) {
+    const auto target = NativePath(path);
+#ifndef _WIN32
+    if (replaceFile(target, path, data, size))
+        return;
+#endif
+    if (!writeFile(target, path, mode, data, size))
+        throw Domain::RelinkerException("Cannot open output file: " + path);
+}
+
+}
+
+void FileWriter::Write(const std::string& path, const std::vector<std::uint8_t>& data) {
+    writeOutput(path, std::ios::binary, reinterpret_cast<const char*>(data.data()), data.size());
 }
 
 void FileWriter::Write(const std::string& path, const std::string& content) {
-    std::ofstream f(NativePath(path));
-    if (!f)
-        throw Domain::RelinkerException("Cannot open output file: " + path);
-    f << content;
-    f.close();
-    if (!f)
-        throw Domain::RelinkerException("Failed to write file: " + path);
+    writeOutput(path, std::ios::out, content.data(), content.size());
 }
 
 }

core/relinker/io/tests/FileWriterTests.cpp

@@ -1,9 +1,19 @@
 #include <io/FileWriter.hpp>
 #include <domain/Types.hpp>
+#include <csignal>
+#include <filesystem>
+#include <fstream>
 #include <iostream>
+#include <iterator>
 #include <stdexcept>
 #include <string>
+#include <system_error>
 #include <vector>
+#include <fcntl.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
 
 namespace {
 
@@ -21,6 +31,141 @@ void requireWriteFailure(const TValue& data) {
     throw std::runtime_error("Write to /dev/full reported success");
 }
 
+std::string readFile(const std::filesystem::path& path) {
+    std::ifstream file(path, std::ios::binary);
+    return {std::istreambuf_iterator<char>(file), std::istreambuf_iterator<char>()};
+}
+
+std::vector<std::string> siblings(const std::filesystem::path& output) {
+    std::vector<std::string> names;
+    for (const auto& entry : std::filesystem::directory_iterator(output.parent_path())) {
+        const auto name = entry.path().filename().string();
+        if (name.starts_with(output.filename().string() + "."))
+            names.push_back(name);
+    }
+    return names;
+}
+
+template<typename TValue>
+void requireRunningReplacement(const std::filesystem::path& directory, const TValue& data, const bool leftover) {
+    const auto output = directory / (leftover ? "leftover" : "running");
+    if (leftover)
+        std::ofstream(directory / "leftover.tmp", std::ios::binary) << "killed";
+    std::filesystem::copy_file("/bin/sleep", output);
+    const auto permissions = std::filesystem::perms::owner_all | std::filesystem::perms::group_all;
+    std::filesystem::permissions(output, permissions);
+    const auto previous = readFile(output);
+    const pid_t child = fork();
+    if (child < 0)
+        throw std::runtime_error("Cannot start the previous executable");
+    if (child == 0) {
+        execl(output.c_str(), "sleep", "30", nullptr);
+        _exit(127);
+    }
+    std::error_code error;
+    bool started = false;
+    for (int attempt = 0; attempt < 2000 && !started; ++attempt) {
+        started = std::filesystem::read_symlink("/proc/" + std::to_string(child) + "/exe", error) == output;
+        if (!started)
+            usleep(1000);
+    }
+    const int descriptor = open(output.c_str(), O_RDONLY);
+    std::string failure;
+    if (started) {
+        const auto mask = umask(077);
+        try {
+            Io::FileWriter().Write(output.string(), data);
+        } catch (const std::exception& e) {
+            failure = e.what();
+        }
+        umask(mask);
+    }
+    std::string kept(previous.size(), '\0');
+    const bool keptPrevious = pread(descriptor, kept.data(), kept.size(), 0) == static_cast<ssize_t>(kept.size()) && kept == previous;
+    close(descriptor);
+    const bool running = waitpid(child, nullptr, WNOHANG) == 0;
+    if (running) {
+        kill(child, SIGKILL);
+        waitpid(child, nullptr, 0);
+    }
+    if (!started)
+        throw std::runtime_error("The previous executable did not start");
+    if (!failure.empty())
+        throw std::runtime_error("Replacing a running executable failed: " + failure);
+    if (readFile(output) != std::string(data.begin(), data.end()))
+        throw std::runtime_error("Running executable was not replaced");
+    if ((std::filesystem::status(output).permissions() & std::filesystem::perms::all) != permissions)
+        throw std::runtime_error("Replacement did not keep the previous permissions");
+    if (!keptPrevious)
+        throw std::runtime_error("An open descriptor no longer reads the previous file");
+    if (!running)
+        throw std::runtime_error("Running executable stopped");
+    if (siblings(output) != (leftover ? std::vector<std::string>{"leftover.tmp"} : std::vector<std::string>{}))
+        throw std::runtime_error("Temporary file left behind");
+    if (leftover && readFile(directory / "leftover.tmp") != "killed")
+        throw std::runtime_error("A file left by an earlier run was changed");
+}
+
+template<typename TValue>
+void requireFailedReplacement(const std::filesystem::path& directory, const TValue& data) {
+    const auto output = directory / "limited";
+    std::ofstream(output, std::ios::binary) << "previous";
+    rlimit limit{};
+    getrlimit(RLIMIT_FSIZE, &limit);
+    rlimit lowered = limit;
+    lowered.rlim_cur = 1;
+    const auto handler = std::signal(SIGXFSZ, SIG_IGN);
+    setrlimit(RLIMIT_FSIZE, &lowered);
+    std::string failure;
+    try {
+        Io::FileWriter().Write(output.string(), data);
+    } catch (const Domain::RelinkerException& e) {
+        failure = e.what();
+    }
+    setrlimit(RLIMIT_FSIZE, &limit);
+    std::signal(SIGXFSZ, handler);
+    if (failure != "Failed to write file: " + output.string())
+        throw std::runtime_error("Write past the file size limit did not fail: " + failure);
+    if (readFile(output) != "previous" || !siblings(output).empty())
+        throw std::runtime_error("Failed write did not keep the previous file");
+}
+
+template<typename TValue>
+void requireInPlaceWrites(const std::filesystem::path& directory, const TValue& data) {
+    const std::string expected(data.begin(), data.end());
+    const auto target = directory / "target";
+    const auto link = directory / "link";
+    std::ofstream(target, std::ios::binary) << "previous";
+    std::filesystem::create_symlink(target, link);
+    Io::FileWriter().Write(link.string(), data);
+    if (!std::filesystem::is_symlink(link) || readFile(target) != expected)
+        throw std::runtime_error("Symbolic link output was not written through");
+    const auto reserved = directory / "reserved";
+    std::ofstream(reserved, std::ios::binary) << "previous";
+    std::ofstream(directory / "reserved.tmp", std::ios::binary) << "unrelated";
+    Io::FileWriter().Write(reserved.string(), data);
+    if (readFile(reserved) != expected || readFile(directory / "reserved.tmp") != "unrelated")
+        throw std::runtime_error("Existing temporary file name was not left alone");
+}
+
+template<typename TValue>
+void requireReplacement(const TValue& data) {
+    const auto directory = std::filesystem::canonical(std::filesystem::temp_directory_path()) / ("anyps5-file-writer-" + std::to_string(getpid()));
+    std::error_code error;
+    std::filesystem::remove_all(directory, error);
+    std::filesystem::create_directory(directory);
+    try {
+        requireRunningReplacement(directory, data, false);
+        requireRunningReplacement(directory, data, true);
+        requireFailedReplacement(directory, data);
+        requireInPlaceWrites(directory, data);
+    } catch (...) {
+        std::filesystem::remove_all(directory, error);
+        throw;
+    }
+    std::filesystem::remove_all(directory, error);
+}
+
 }
 
 int main(int argc, char* argv[]) {
@@ -28,12 +173,17 @@ int main(int argc, char* argv[]) {
         if (argc != 2) throw std::runtime_error("Expected binary or text");
         const std::string mode = argv[1];
         for (const std::size_t size : {std::size_t{3}, std::size_t{65536}}) {
-            if (mode == "binary")
-                requireWriteFailure(std::vector<std::uint8_t>(size, 0x41));
-            else if (mode == "text")
-                requireWriteFailure(std::string(size, 'A'));
-            else
+            if (mode == "binary") {
+                const std::vector<std::uint8_t> data(size, 0x41);
+                requireWriteFailure(data);
+                requireReplacement(data);
+            } else if (mode == "text") {
+                const std::string data(size, 'A');
+                requireWriteFailure(data);
+                requireReplacement(data);
+            } else {
                 throw std::runtime_error("Unknown test mode: " + mode);
+            }
         }
     } catch (const std::exception& e) {
         std::cerr << e.what() << '\n';