From 8748fbd542a00ab7bbfbf17210b29f8d51c3c4b5 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:23:37 -0500 Subject: file_sys: decompress large NUP zstd members once 31b901b93b only cached a zstd member while it fit the 512 MiB cache. Anything larger was never cached, so every Read() allocated the full uncompressed size and decompressed the whole extent again, which makes a multi-GB RomFS read in small chunks unusably slow. Members between 256 and 512 MiB evicted each other on alternate reads, and Insert copied the vector so memory briefly doubled. Two threads decompressing the same member also left a duplicate LRU entry and a wrong byte count. The writer emits one zstd frame per member, so there are no frame boundaries to seek by. Members larger than half the cache are now streamed once into a temporary file under the cache directory and read from there, using bounded memory. The file is removed when the container is released. Smaller members stay in the LRU, which now moves the vector in and ignores a duplicate insert. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/file_sys/nup_container.cpp | 114 +++++++++++++++++++++++++++++++++--- 1 file changed, 107 insertions(+), 7 deletions(-) (limited to 'src/core/file_sys/nup_container.cpp') diff --git a/src/core/file_sys/nup_container.cpp b/src/core/file_sys/nup_container.cpp index 0e485b44ec..28e4a6ac89 100644 --- a/src/core/file_sys/nup_container.cpp +++ b/src/core/file_sys/nup_container.cpp @@ -8,11 +8,15 @@ #include #include #include +#include +#include #include +#include #include #include +#include "common/fs/path_util.h" #include "common/logging.h" #include "common/swap.h" #include "core/file_sys/vfs/vfs_offset.h" @@ -142,8 +146,8 @@ bool Sha256(const u8* data, std::size_t size, std::array& out) { return length == out.size(); } -/// A member file whose extent is zstd-compressed. Decompresses on first read -/// and caches the result (LRU-evicted) unless the file exceeds the cache cap. +/// A member file whose extent is zstd-compressed. Decompresses on first read and caches the +/// result (LRU-evicted); a file over half the cache cap is decompressed once to a temporary file. class NupVfsFile final : public VfsFile { public: NupVfsFile(VirtualFile backing_, u64 extent_offset_, u64 extent_size_, u64 uncompressed_size_, @@ -151,7 +155,7 @@ public: std::size_t cache_index_) : backing(std::move(backing_)), extent_offset(extent_offset_), extent_size(extent_size_), uncompressed_size(uncompressed_size_), name(std::move(name_)), cache(std::move(cache_)), - cache_index(cache_index_), cache_enabled(uncompressed_size <= NupDecompressCache::DecompressCacheCap) {} + cache_index(cache_index_), cache_enabled(uncompressed_size <= cache->cap / 2) {} std::string GetName() const override { return name; @@ -183,6 +187,10 @@ public: } length = std::min(length, static_cast(uncompressed_size) - offset); + if (!cache_enabled) { + return ReadSpilled(data, length, offset); + } + { std::scoped_lock lock{cache->mutex}; if (const auto* cached = cache->Find(cache_index)) { @@ -198,9 +206,7 @@ public: } { std::scoped_lock lock{cache->mutex}; - if (cache_enabled) { - cache->Insert(cache_index, scratch); - } + cache->Insert(cache_index, scratch); if (const auto* cached = cache->Find(cache_index)) { std::memcpy(data, cached->data() + offset, length); return length; @@ -220,6 +226,88 @@ public: } private: + std::size_t ReadSpilled(u8* data, std::size_t length, std::size_t offset) const { + std::shared_ptr spill; + { + std::scoped_lock lock{cache->mutex}; + spill = cache->GetSpill(cache_index); + } + std::scoped_lock lock{spill->mutex}; + if (!spill->attempted) { + spill->attempted = true; + DecompressToFile(*spill); + } + if (!spill->file.IsOpen() || !spill->file.Seek(static_cast(offset))) { + return 0; + } + return spill->file.ReadSpan(std::span{data, length}); + } + + void DecompressToFile(NupSpillFile& spill) const { + const auto fail = [&](const char* reason) { + LOG_ERROR(Loader, "NUP zstd decompression of {} to a temporary file failed: {}", name, + reason); + spill.file.Close(); + if (!spill.path.empty()) { + std::error_code ec; + std::filesystem::remove(spill.path, ec); + } + }; + if (extent_size > backing->GetSize() || extent_offset > backing->GetSize() - extent_size) { + return fail("extent out of range"); + } + + std::random_device random; + std::error_code ec; + std::filesystem::create_directories(cache->spill_dir, ec); + spill.path = + cache->spill_dir / fmt::format("{:08x}{:08x}-{}.tmp", random(), random(), cache_index); + Common::FS::IOFile out{spill.path, Common::FS::FileAccessMode::Write}; + if (!out.IsOpen()) { + return fail("cannot create the temporary file"); + } + + std::unique_ptr dctx{ZSTD_createDCtx(), + &ZSTD_freeDCtx}; + std::vector in_buffer(ZSTD_DStreamInSize()); + std::vector out_buffer(ZSTD_DStreamOutSize()); + u64 read = 0; + u64 written = 0; + bool frame_done = false; + while (!frame_done && read < extent_size) { + const auto chunk = + static_cast(std::min(in_buffer.size(), extent_size - read)); + if (backing->Read(in_buffer.data(), chunk, + static_cast(extent_offset + read)) != chunk) { + return fail("short read of the compressed data"); + } + read += chunk; + ZSTD_inBuffer input{in_buffer.data(), chunk, 0}; + ZSTD_outBuffer output{}; + do { + output = {out_buffer.data(), out_buffer.size(), 0}; + const std::size_t result = ZSTD_decompressStream(dctx.get(), &output, &input); + if (ZSTD_isError(result) || output.pos > uncompressed_size - written) { + return fail("corrupt or oversized data"); + } + if (out.WriteSpan(std::span{out_buffer.data(), output.pos}) != + output.pos) { + return fail("cannot write the temporary file"); + } + written += output.pos; + frame_done = result == 0; + } while (!frame_done && (input.pos < input.size || output.pos == output.size)); + } + if (!frame_done || written != uncompressed_size) { + return fail("size mismatch"); + } + out.Close(); + spill.file.Open(spill.path, Common::FS::FileAccessMode::Read); + if (!spill.file.IsOpen()) { + fail("cannot reopen the temporary file"); + } + } + std::vector Decompress() const { if (extent_size > backing->GetSize() || extent_offset > backing->GetSize() - extent_size) { return {}; @@ -251,6 +339,13 @@ private: } // namespace +NupSpillFile::~NupSpillFile() { + file.Close(); + if (!path.empty()) { + std::error_code ec; + std::filesystem::remove(path, ec); + } +} bool NupContainer::IsNupFile(const VirtualFile& file) { if (file == nullptr || file->GetSize() < SuperblockSize) { @@ -261,8 +356,13 @@ bool NupContainer::IsNupFile(const VirtualFile& file) { return magic == HeaderMagic; } -NupContainer::NupContainer(VirtualFile file_) +NupContainer::NupContainer(VirtualFile file_, std::size_t memory_cache_cap, + std::filesystem::path spill_dir_) : file(std::move(file_)), cache(std::make_shared()) { + cache->cap = memory_cache_cap; + cache->spill_dir = spill_dir_.empty() + ? Common::FS::GetEdenPath(Common::FS::EdenPath::CacheDir) / "nup" + : std::move(spill_dir_); if (file == nullptr) { return; } -- cgit v1.2.3 From 089105ab44c87f1ef0526221144a1979c56558e6 Mon Sep 17 00:00:00 2001 From: Sssnowyyy <78863999+Sssnowyyy@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:25:26 -0500 Subject: file_sys: reject NUP tables with wrapping bounds or cyclic directories 31b901b93b checked the TOC with unchecked arithmetic on counts and offsets read from the file. The record-table size (u64 count times record size) and u32 first_child + child_count can wrap, so the bounds checks pass and later reads go out of bounds; the same holds for the TOC range, name ranges, file extents and the meta range. A directory that lists its own ancestor as a child also made BuildDirTree recurse until the stack overflowed. Bound the counts by the TOC size before multiplying, compare ranges by subtraction, and widen the child range to u64. Walk the directory graph once after parsing with an explicit stack and reject cycles and nesting deeper than 128, so BuildDirTree can no longer recurse without bound. The file is rejected with ErrorNUPBadFormat. Assisted-by: Claude Sonnet 5.5 (implementation and commit message) --- src/core/file_sys/nup_container.cpp | 96 +++++++++++++++++++++++++++++++------ src/core/file_sys/nup_container.h | 1 + src/tests/core/nup_writer.cpp | 62 ++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 15 deletions(-) (limited to 'src/core/file_sys/nup_container.cpp') diff --git a/src/core/file_sys/nup_container.cpp b/src/core/file_sys/nup_container.cpp index 28e4a6ac89..27ac4486ca 100644 --- a/src/core/file_sys/nup_container.cpp +++ b/src/core/file_sys/nup_container.cpp @@ -35,6 +35,7 @@ constexpr u32 TocMagic = 0x01434F54; // "TOC\x01" little-endian constexpr u8 CodecRaw = 0; constexpr u8 CodecZstd = 1; constexpr u8 PackageFlagStandaloneUpdate = 0x01; +constexpr u32 MaxDirDepth = 128; #pragma pack(push, 1) struct Superblock { @@ -393,9 +394,9 @@ bool NupContainer::Parse() { toc_size = super.toc_size; blob_offset = super.blob_offset; file_size = super.file_size; - if (file_size != file->GetSize() || toc_offset < SuperblockSize || - blob_offset < toc_offset + toc_size || blob_offset % 0x10 != 0 || - blob_offset > file_size) { + if (file_size != file->GetSize() || toc_offset < SuperblockSize || blob_offset > file_size || + toc_offset > blob_offset || toc_size > blob_offset - toc_offset || + blob_offset % 0x10 != 0) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -425,15 +426,22 @@ bool NupContainer::Parse() { return false; } + // The counts come from the file, so bound them by the TOC size before multiplying. + if (header.dir_count > toc.size() / DirRecordSize || + header.file_count > toc.size() / FileRecordSize) { + status = Loader::ResultStatus::ErrorNUPBadFormat; + return false; + } const u64 needed = TocHeaderSize + static_cast(header.package_count) * PackageRecordSize + header.dir_count * DirRecordSize + header.file_count * FileRecordSize; - const u64 children_offset = needed; - const u64 children_bytes = header.pool_offset - needed; - if (toc.size() < needed || header.pool_offset < needed || - header.pool_offset + header.pool_size > toc.size() || children_bytes % sizeof(u32) != 0) { + if (needed > toc.size() || header.pool_offset < needed || header.pool_offset > toc.size() || + header.pool_size > toc.size() - header.pool_offset || + (header.pool_offset - needed) % sizeof(u32) != 0) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } + const u64 children_offset = needed; + const u64 children_bytes = header.pool_offset - needed; const u8* base = toc.data(); std::size_t pos = TocHeaderSize; @@ -458,8 +466,8 @@ bool NupContainer::Parse() { package.root_dir = record.root_dir; package.exefs_dir = record.exefs_dir; package.romfs_dir = record.romfs_dir; - if (record.meta_size > 0 && record.meta_offset != 0 && - record.meta_offset + record.meta_size <= file_size) { + if (record.meta_size > 0 && record.meta_offset != 0 && record.meta_offset <= file_size && + record.meta_size <= file_size - record.meta_offset) { package.meta_offset = record.meta_offset; package.meta_size = record.meta_size; } @@ -471,8 +479,9 @@ bool NupContainer::Parse() { DirRecord record{}; std::memcpy(&record, base + pos, sizeof(record)); pos += DirRecordSize; - if (record.first_child + record.child_count > children_bytes / sizeof(u32) || - record.name_offset + record.name_length > header.pool_size) { + if (static_cast(record.first_child) + record.child_count > + children_bytes / sizeof(u32) || + static_cast(record.name_offset) + record.name_length > header.pool_size) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -492,9 +501,9 @@ bool NupContainer::Parse() { (record.size == 0 && (record.extent_offset != 0 || record.extent_size != 0)) || (record.size != 0 && (record.extent_size == 0 || record.extent_offset < blob_offset || - record.extent_offset % 0x10 != 0 || - record.extent_offset + record.extent_size > file_size)) || - record.name_offset + record.name_length > header.pool_size) { + record.extent_offset % 0x10 != 0 || record.extent_offset > file_size || + record.extent_size > file_size - record.extent_offset)) || + static_cast(record.name_offset) + record.name_length > header.pool_size) { status = Loader::ResultStatus::ErrorNUPBadFormat; return false; } @@ -517,6 +526,11 @@ bool NupContainer::Parse() { } } + if (!ValidateDirTree()) { + status = Loader::ResultStatus::ErrorNUPBadFormat; + return false; + } + for (auto& package : packages) { if (!ParseMeta(package)) { status = Loader::ResultStatus::ErrorNUPBadFormat; @@ -526,6 +540,57 @@ bool NupContainer::Parse() { return true; } +bool NupContainer::ValidateDirTree() const { + enum : u8 { Unvisited, InProgress, Done }; + struct Frame { + u32 dir; + u32 next; + }; + std::vector state(dirs.size(), Unvisited); + std::vector height(dirs.size(), 0); + std::vector stack; + for (u32 start = 0; start < dirs.size(); ++start) { + if (state[start] != Unvisited) { + continue; + } + state[start] = InProgress; + stack.push_back({start, 0}); + while (!stack.empty()) { + const Frame frame = stack.back(); + const DirNode& node = dirs[frame.dir]; + if (frame.next == node.child_count) { + state[frame.dir] = Done; + stack.pop_back(); + if (!stack.empty()) { + u32& parent_height = height[stack.back().dir]; + parent_height = std::max(parent_height, height[frame.dir] + 1); + } + continue; + } + ++stack.back().next; + const u32 child = children[static_cast(node.first_child) + frame.next]; + if (child >= dirs.size()) { + continue; + } + if (state[child] == InProgress) { + LOG_ERROR(Loader, "NUP directory tree contains a cycle"); + return false; + } + if (state[child] == Done) { + height[frame.dir] = std::max(height[frame.dir], height[child] + 1); + } else { + state[child] = InProgress; + stack.push_back({child, 0}); + } + if (stack.size() + height[child] > MaxDirDepth) { + LOG_ERROR(Loader, "NUP directory tree is nested too deeply"); + return false; + } + } + } + return true; +} + bool NupContainer::ParseMeta(PackageInfo& package) { if (package.meta_offset == 0) { return true; @@ -647,7 +712,8 @@ VirtualFile NupContainer::OpenFile(std::size_t file_index) { void NupContainer::BuildDirTree(u32 dir_index, VectorVfsDirectory& out) { const DirNode& node = dirs[dir_index]; - for (u32 slot = node.first_child; slot < node.first_child + node.child_count; ++slot) { + const u64 end = static_cast(node.first_child) + node.child_count; + for (u64 slot = node.first_child; slot < end; ++slot) { const u32 child = children[slot]; if (child >= dirs.size()) { out.AddFile(OpenFile(child - static_cast(dirs.size()))); diff --git a/src/core/file_sys/nup_container.h b/src/core/file_sys/nup_container.h index 0980c25517..8787717d47 100644 --- a/src/core/file_sys/nup_container.h +++ b/src/core/file_sys/nup_container.h @@ -184,6 +184,7 @@ private: bool Parse(); bool ParseMeta(PackageInfo& package); + bool ValidateDirTree() const; void BuildDirTree(u32 dir_index, VectorVfsDirectory& out); VirtualDir BuildPackageDir(u32 dir_index); std::vector ReadExtentRaw(const FileNode& node) const; diff --git a/src/tests/core/nup_writer.cpp b/src/tests/core/nup_writer.cpp index 5dec0134cb..48e14daaef 100644 --- a/src/tests/core/nup_writer.cpp +++ b/src/tests/core/nup_writer.cpp @@ -19,6 +19,7 @@ #include "core/file_sys/nup_container.h" #include "core/file_sys/nup_writer.h" #include "core/file_sys/vfs/vfs_real.h" +#include "core/file_sys/vfs/vfs_vector.h" namespace { @@ -173,6 +174,23 @@ void MakeDump(const fs::path& root) { Put(root / "dlc/0400ABCD00011001/0/romfs/addon.bin", "addon data"); } +void SetLe(std::string& buf, size_t at, u64 value, size_t bytes) { + for (size_t i = 0; i < bytes; ++i) { + buf[at + i] = static_cast(value >> (8 * i)); + } +} + +// Applies `patch` to a valid archive, fixes up the TOC hash and opens the result. +template +Loader::ResultStatus StatusAfterPatch(std::string nup, Patch&& patch) { + const u64 toc = Le(nup, 0x08, 8); + patch(nup, toc); + const auto hash = Sha256(std::string_view{nup}.substr(toc, Le(nup, 0x10, 8))); + std::copy(hash.begin(), hash.end(), nup.begin() + 0x18); + const auto data = std::vector(nup.begin(), nup.end()); + return NupContainer{std::make_shared(data, "bad.nup")}.GetStatus(); +} + } // namespace TEST_CASE("NUP packs base, updates and add-ons and reads back through NupContainer", "[nup]") { @@ -305,6 +323,50 @@ TEST_CASE("NUP reads zstd members above the memory cache through a temporary fil REQUIRE(std::distance(fs::directory_iterator(spill_dir), fs::directory_iterator{}) == 0); } +TEST_CASE("NUP rejects a TOC whose counts or offsets wrap", "[nup]") { + TempDir t; + const auto root = t.path / "Game"; + MakeDump(root); + const auto archive = t.path / "Game.nup"; + REQUIRE(FileSys::Nup::PackDump(root, archive) == FileSys::Nup::PackResult::Success); + const auto nup = ReadAll(archive); + const auto no_patch = [](std::string&, u64) {}; + REQUIRE(StatusAfterPatch(nup, no_patch) == Loader::ResultStatus::Success); + + // 2^61 directories wrap dir_count * 0x18 back to the real record size. + REQUIRE(StatusAfterPatch(nup, [](std::string& buf, u64 toc) { + SetLe(buf, toc + 0x08, Le(buf, toc + 0x08, 8) + (1ULL << 61), 8); + }) == Loader::ResultStatus::ErrorNUPBadFormat); + + const auto first_dir = [](const std::string& buf, u64 toc) { + return toc + 0x30 + Le(buf, toc + 0x04, 4) * 0x40; + }; + // first_child + child_count wraps to a small value. + REQUIRE(StatusAfterPatch(nup, [&](std::string& buf, u64 toc) { + SetLe(buf, first_dir(buf, toc) + 0x08, 0xFFFFFFFF, 4); + SetLe(buf, first_dir(buf, toc) + 0x0C, 2, 4); + }) == Loader::ResultStatus::ErrorNUPBadFormat); +} + +TEST_CASE("NUP rejects a directory that contains its own ancestor", "[nup]") { + TempDir t; + const auto root = t.path / "Game"; + MakeDump(root); + const auto archive = t.path / "Game.nup"; + REQUIRE(FileSys::Nup::PackDump(root, archive) == FileSys::Nup::PackResult::Success); + + // Give the base RomFS directory the same children as the package root, which lists it. + REQUIRE(StatusAfterPatch(ReadAll(archive), [](std::string& buf, u64 toc) { + const u64 dirs = toc + 0x30 + Le(buf, toc + 0x04, 4) * 0x40; + const u64 root_dir = Le(buf, toc + 0x30 + 0x08, 4); + const u64 romfs_dir = Le(buf, toc + 0x30 + 0x10, 4); + const u64 root_record = dirs + root_dir * 0x18; + const u64 romfs_record = dirs + romfs_dir * 0x18; + SetLe(buf, romfs_record + 0x08, Le(buf, root_record + 0x08, 4), 4); + SetLe(buf, romfs_record + 0x0C, Le(buf, root_record + 0x0C, 4), 4); + }) == Loader::ResultStatus::ErrorNUPBadFormat); +} + TEST_CASE("NUP marks updates standalone when the dump has no base", "[nup]") { TempDir t; const auto root = t.path / "Game"; -- cgit v1.2.3