From 05cccb585d7c248784e547ae65ca595823c385c7 Mon Sep 17 00:00:00 2001 From: PabloMK7 Date: Thu, 16 May 2024 00:03:40 +0200 Subject: [PATCH] Artic Base: Fix out of bounds cache reads (#127) --- src/common/static_lru_cache.h | 8 +++++ src/core/file_sys/artic_cache.cpp | 59 +++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/src/common/static_lru_cache.h b/src/common/static_lru_cache.h index bd692e94e..46c3ce4de 100644 --- a/src/common/static_lru_cache.h +++ b/src/common/static_lru_cache.h @@ -88,8 +88,16 @@ public: } } + void invalidate(const key_type& key) { + auto i = find(key); + if (i != m_list.cend()) { + m_list.erase(i); + } + } + void clear() { m_list.clear(); + m_array.fill(value_type{}); } private: diff --git a/src/core/file_sys/artic_cache.cpp b/src/core/file_sys/artic_cache.cpp index b5c963495..7ce2f0346 100644 --- a/src/core/file_sys/artic_cache.cpp +++ b/src/core/file_sys/artic_cache.cpp @@ -25,13 +25,20 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std auto res = ReadFromArtic(file_handle, reinterpret_cast(big_cache_entry.second.data()), length, offset); - if (res.Failed()) + if (res.Failed()) { + big_cache.invalidate(std::make_pair(offset, length)); return res; - length = res.Unwrap(); + } + read_progress = res.Unwrap(); } else { - LOG_TRACE(Service_FS, "ArticCache BHIT: offset={}, length={}", offset, length); + LOG_TRACE(Service_FS, "ArticCache BHIT: offset={}, length={}", offset, + read_progress); + } + memcpy(buffer, big_cache_entry.second.data(), read_progress); + if (read_progress < length) { + // Invalidate the entry as it is not fully read + big_cache.invalidate(std::make_pair(offset, length)); } - memcpy(buffer, big_cache_entry.second.data(), length); } else { if (segments[0].second < very_big_cache_skip) { std::unique_lock very_big_read_guard(very_big_cache_mutex); @@ -44,28 +51,39 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std auto res = ReadFromArtic( file_handle, reinterpret_cast(very_big_cache_entry.second.data()), length, offset); - if (res.Failed()) + if (res.Failed()) { + very_big_cache.invalidate(std::make_pair(offset, length)); return res; - length = res.Unwrap(); + } + read_progress = res.Unwrap(); } else { - LOG_TRACE(Service_FS, "ArticCache VBHIT: offset={}, length={}", offset, length); + LOG_TRACE(Service_FS, "ArticCache VBHIT: offset={}, length={}", offset, + read_progress); + } + memcpy(buffer, very_big_cache_entry.second.data(), read_progress); + if (read_progress < length) { + // Invalidate the entry as it is not fully read + very_big_cache.invalidate(std::make_pair(offset, length)); } - memcpy(buffer, very_big_cache_entry.second.data(), length); } else { LOG_TRACE(Service_FS, "ArticCache SKIP: offset={}, length={}", offset, length); auto res = ReadFromArtic(file_handle, buffer, length, offset); if (res.Failed()) return res; - length = res.Unwrap(); + read_progress = res.Unwrap(); } } - return length; + return read_progress; } // TODO(PabloMK7): Make cache thread safe, read the comment in CacheReady function. std::unique_lock read_guard(cache_mutex); + bool read_past_end = false; for (const auto& seg : segments) { + if (read_past_end) { + break; + } std::size_t read_size = cache_line_size; std::size_t page = OffsetToPage(seg.first); // Check if segment is in cache @@ -73,9 +91,28 @@ ResultVal ArticCache::Read(s32 file_handle, std::size_t offset, std if (!cache_entry.first) { // If not found, read from artic and cache the data auto res = ReadFromArtic(file_handle, cache_entry.second.data(), read_size, page); - if (res.Failed()) + if (res.Failed()) { + // Invalidate the requested entry as it is not populated + cache.invalidate(page); + + // In the very unlikely case the file size is a multiple of the cache size, + // and the game request more data than the file size, this will save us from + // returning an incorrect out of bounds error caused by reading at just the very end + // of the file. + constexpr u32 out_of_bounds_read = 714; + if (res.Code().description == out_of_bounds_read) { + return read_progress; + } return res; + } + size_t expected_read_size = read_size; read_size = res.Unwrap(); + // + if (read_size < expected_read_size) { + // Invalidate the requested entry as it is not fully read + cache.invalidate(page); + read_past_end = true; + } LOG_TRACE(Service_FS, "ArticCache MISS: page={}, length={}, into={}", page, seg.second, (seg.first - page)); } else {