From 30e3d795da462bde55477f6f067bb77079ea7ceb Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Thu, 13 Jun 2024 15:15:15 -0700 Subject: [PATCH] FEX: Consolidate JSON allocators and fix 3691 Fixes #3691 We weren't checking if the file was empty before using its `at` function member. This was causing an early crash if the config file existed but was empty. Consolidates the three locations that copy and pasted the json allocator tools and adds an empty check for all of them. Also adds two missing checks to the ThunksDB handler that could have resulted in the same crash if ThunksDB was an empty file. --- Source/Common/CMakeLists.txt | 1 + Source/Common/Config.cpp | 28 ++--------- Source/Common/JSONPool.cpp | 23 +++++++++ Source/Common/JSONPool.h | 25 ++++++++++ Source/Tools/FEXRootFSFetcher/Main.cpp | 28 ++--------- .../LinuxSyscalls/FileManagement.cpp | 48 +++++-------------- 6 files changed, 67 insertions(+), 86 deletions(-) create mode 100644 Source/Common/JSONPool.cpp create mode 100644 Source/Common/JSONPool.h diff --git a/Source/Common/CMakeLists.txt b/Source/Common/CMakeLists.txt index a5dcb9c46..112b8c0c1 100644 --- a/Source/Common/CMakeLists.txt +++ b/Source/Common/CMakeLists.txt @@ -5,6 +5,7 @@ set(SRCS Config.cpp ArgumentLoader.cpp EnvironmentLoader.cpp + JSONPool.cpp StringUtil.cpp) if (NOT MINGW_BUILD) diff --git a/Source/Common/Config.cpp b/Source/Common/Config.cpp index 9c0ceebe7..093e11682 100644 --- a/Source/Common/Config.cpp +++ b/Source/Common/Config.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: MIT #include "Common/ArgumentLoader.h" #include "Common/Config.h" +#include "Common/JSONPool.h" #include #include @@ -22,38 +23,15 @@ namespace FEX::Config { namespace JSON { - struct JsonAllocator { - jsonPool_t PoolObject; - fextl::unique_ptr> json_objects; - }; - static_assert(offsetof(JsonAllocator, PoolObject) == 0, "This needs to be at offset zero"); - - json_t* PoolInit(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - alloc->json_objects = fextl::make_unique>(); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); - } - - json_t* PoolAlloc(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); - } - static void LoadJSonConfig(const fextl::string& Config, std::function Func) { fextl::vector Data; if (!FEXCore::FileLoading::LoadFile(Data, Config)) { return; } - JsonAllocator Pool { - .PoolObject = - { - .init = PoolInit, - .alloc = PoolAlloc, - }, - }; + FEX::JSON::JsonAllocator Pool {}; + const json_t* json = FEX::JSON::CreateJSON(Data, Pool); - const json_t* json = json_createWithPool(&Data.at(0), &Pool.PoolObject); if (!json) { LogMan::Msg::EFmt("Couldn't create json"); return; diff --git a/Source/Common/JSONPool.cpp b/Source/Common/JSONPool.cpp new file mode 100644 index 000000000..9d23182bc --- /dev/null +++ b/Source/Common/JSONPool.cpp @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: MIT +#include "Common/JSONPool.h" + +namespace FEX::JSON { +json_t* PoolInit(jsonPool_t* Pool); +json_t* PoolAlloc(jsonPool_t* Pool); + +JsonAllocator::JsonAllocator() + : PoolObject { + .init = FEX::JSON::PoolInit, + .alloc = FEX::JSON::PoolAlloc, + } {} + +json_t* PoolInit(jsonPool_t* Pool) { + JsonAllocator* alloc = reinterpret_cast(Pool); + return &*alloc->json_objects.emplace(alloc->json_objects.end()); +} + +json_t* PoolAlloc(jsonPool_t* Pool) { + JsonAllocator* alloc = reinterpret_cast(Pool); + return &*alloc->json_objects.emplace(alloc->json_objects.end()); +} +} // namespace FEX::JSON diff --git a/Source/Common/JSONPool.h b/Source/Common/JSONPool.h new file mode 100644 index 000000000..4596a84b2 --- /dev/null +++ b/Source/Common/JSONPool.h @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: MIT + +#include +#include + +#include + +namespace FEX::JSON { +struct JsonAllocator { + jsonPool_t PoolObject; + fextl::list json_objects; + + JsonAllocator(); +}; +static_assert(offsetof(JsonAllocator, PoolObject) == 0, "This needs to be at offset zero"); + +template +const json_t* CreateJSON(T& Container, JsonAllocator& Allocator) { + if (Container.empty()) { + return nullptr; + } + + return json_createWithPool(&Container.at(0), &Allocator.PoolObject); +} +} // namespace FEX::JSON diff --git a/Source/Tools/FEXRootFSFetcher/Main.cpp b/Source/Tools/FEXRootFSFetcher/Main.cpp index 77864144a..c6145f6c0 100644 --- a/Source/Tools/FEXRootFSFetcher/Main.cpp +++ b/Source/Tools/FEXRootFSFetcher/Main.cpp @@ -3,6 +3,7 @@ #include #include "Common/cpp-optparse/OptionParser.h" +#include "Common/JSONPool.h" #include "XXFileHash.h" #include "Common/ArgumentLoader.h" @@ -510,23 +511,6 @@ bool DownloadToPathWithZenityProgress(const fextl::string& URL, const fextl::str return Exec::ExecAndWaitForResponse(ExecveArgs[0], const_cast(ExecveArgs.data())) == 0; } -struct JsonAllocator { - jsonPool_t PoolObject; - std::unique_ptr> json_objects; -}; -static_assert(offsetof(JsonAllocator, PoolObject) == 0, "This needs to be at offset zero"); - -json_t* PoolInit(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - alloc->json_objects = std::make_unique>(); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); -} - -json_t* PoolAlloc(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); -} - std::optional> GetRootFSLinks() { // Decode the filetargets std::string Data = DownloadToString(DownloadURL); @@ -535,15 +519,9 @@ std::optional> GetRootFSLinks() { return std::nullopt; } - JsonAllocator Pool { - .PoolObject = - { - .init = PoolInit, - .alloc = PoolAlloc, - }, - }; + FEX::JSON::JsonAllocator Pool {}; + const json_t* json = FEX::JSON::CreateJSON(Data, Pool); - const json_t* json = json_createWithPool(&Data.at(0), &Pool.PoolObject); if (!json) { fprintf(stderr, "Couldn't create json"); return {}; diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/FileManagement.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/FileManagement.cpp index 1842705af..a83284ede 100644 --- a/Source/Tools/LinuxEmulation/LinuxSyscalls/FileManagement.cpp +++ b/Source/Tools/LinuxEmulation/LinuxSyscalls/FileManagement.cpp @@ -8,6 +8,7 @@ $end_info$ #include "Common/Config.h" #include "Common/FDUtils.h" +#include "Common/JSONPool.h" #include "FEXCore/Config/Config.h" #include "LinuxSyscalls/FileManagement.h" @@ -42,25 +43,6 @@ $end_info$ #include -namespace JSON { -struct JsonAllocator { - jsonPool_t PoolObject; - fextl::unique_ptr> json_objects; -}; -static_assert(offsetof(JsonAllocator, PoolObject) == 0, "This needs to be at offset zero"); - -json_t* PoolInit(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - alloc->json_objects = fextl::make_unique>(); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); -} - -json_t* PoolAlloc(jsonPool_t* Pool) { - JsonAllocator* alloc = reinterpret_cast(Pool); - return &*alloc->json_objects->emplace(alloc->json_objects->end()); -} -} // namespace JSON - namespace FEX::HLE { bool FileManager::RootFSPathExists(const char* Filepath) { LOGMAN_THROW_A_FMT(Filepath && Filepath[0] == '/', "Filepath needs to be absolute"); @@ -71,7 +53,6 @@ void FileManager::LoadThunkDatabase(fextl::unordered_map FileData; if (FEXCore::FileLoading::LoadFile(FileData, ThunkDBPath)) { - FileData.push_back(0); // If the thunksDB file exists then we need to check if the rootfs supports multi-arch or not. const bool RootFSIsMultiarch = RootFSPathExists("/usr/lib/x86_64-linux-gnu/") || RootFSPathExists("/usr/lib/i386-linux-gnu/"); @@ -112,15 +93,12 @@ void FileManager::LoadThunkDatabase(fextl::unordered_map FileData; if (FEXCore::FileLoading::LoadFile(FileData, Path)) { - JSON::JsonAllocator Pool { - .PoolObject = - { - .init = JSON::PoolInit, - .alloc = JSON::PoolAlloc, - }, - }; + FEX::JSON::JsonAllocator Pool {}; // If a thunks DB property exists then we pull in data from the thunks database - const json_t* json = json_createWithPool(&FileData.at(0), &Pool.PoolObject); + const json_t* json = FEX::JSON::CreateJSON(FileData, Pool); + if (!json) { + continue; + } + const json_t* ThunksDB = json_getProperty(json, "ThunksDB"); if (!ThunksDB) { continue;