From 6a4eb434f793d47426ff3e8a832c7c85fd4acae3 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Mon, 9 Sep 2024 14:00:44 -0700 Subject: [PATCH 1/2] FEXRootFSFetcher/XXHash: Fix double fd close Since `HadError` is a lambda that handles FD closing this was a double FD close. Just move the xxhash state freeing in there as well and remove the FD and hash freeing. Fixes the double free. --- Source/Tools/FEXRootFSFetcher/XXFileHash.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp b/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp index 66449285a9..0ffb102c13 100644 --- a/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp +++ b/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp @@ -17,8 +17,12 @@ std::pair HashFile(const fextl::string& Filepath) { return {false, 0}; } - auto HadError = [fd]() -> std::pair { + XXH3_state_t* State {}; + auto HadError = [fd, &State]() -> std::pair { close(fd); + if (State) { + XXH3_freeState(State); + } return {false, 0}; }; // Get file size @@ -29,7 +33,7 @@ std::pair HashFile(const fextl::string& Filepath) { lseek(fd, 0, SEEK_SET); // Set up XXHash state - XXH3_state_t* const State = XXH3_createState(); + State = XXH3_createState(); const XXH64_hash_t Seed = 0; if (!State) { @@ -37,8 +41,6 @@ std::pair HashFile(const fextl::string& Filepath) { } if (XXH3_64bits_reset_withSeed(State, Seed) == XXH_ERROR) { - XXH3_freeState(State); - close(fd); return HadError(); } @@ -52,14 +54,10 @@ std::pair HashFile(const fextl::string& Filepath) { ssize_t Result = pread(fd, Data.data(), BLOCK_SIZE, CurrentOffset); if (Result == -1) { - XXH3_freeState(State); - close(fd); return HadError(); } if (XXH3_64bits_update(State, Data.data(), Result) == XXH_ERROR) { - XXH3_freeState(State); - close(fd); return HadError(); } auto Cur = std::chrono::high_resolution_clock::now(); From a05d6ae082026080bf22d297604dc9fec1cde88b Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Mon, 9 Sep 2024 14:07:23 -0700 Subject: [PATCH 2/2] FEXRootFSFetcher/XXHash: Check for errors in two locations that were missed --- Source/Tools/FEXRootFSFetcher/XXFileHash.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp b/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp index 0ffb102c13..2ccd34c4a0 100644 --- a/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp +++ b/Source/Tools/FEXRootFSFetcher/XXFileHash.cpp @@ -27,10 +27,14 @@ std::pair HashFile(const fextl::string& Filepath) { }; // Get file size off_t Size = lseek(fd, 0, SEEK_END); - double SizeD = Size; + if (Size == -1) { + return HadError(); + } // Reset to beginning - lseek(fd, 0, SEEK_SET); + if (lseek(fd, 0, SEEK_SET) == -1) { + return HadError(); + } // Set up XXHash state State = XXH3_createState(); @@ -44,6 +48,7 @@ std::pair HashFile(const fextl::string& Filepath) { return HadError(); } + const double SizeD = Size; std::vector Data(BLOCK_SIZE); off_t CurrentOffset = 0; auto Now = std::chrono::high_resolution_clock::now();