From f16949c2928714537e18e4589f9ec0493d3f729b Mon Sep 17 00:00:00 2001 From: Eladash Date: Wed, 1 Dec 2021 18:09:07 +0200 Subject: [PATCH] fs::file: always use strict reading mode for large reads (#11206) --- Utilities/File.cpp | 2 +- Utilities/File.h | 94 ++++++++++++++++---------------------- rpcs3/Emu/system_utils.cpp | 2 +- rpcs3/Loader/ELF.h | 6 +-- rpcs3/Loader/PSF.cpp | 6 +-- rpcs3/Loader/PUP.cpp | 2 +- rpcs3/Loader/TROPUSR.cpp | 6 +-- rpcs3/Loader/TRP.cpp | 6 +-- 8 files changed, 55 insertions(+), 69 deletions(-) diff --git a/Utilities/File.cpp b/Utilities/File.cpp index 5497e04262..be89628155 100644 --- a/Utilities/File.cpp +++ b/Utilities/File.cpp @@ -1658,7 +1658,7 @@ bool fs::dir::open(const std::string& path) bool fs::file::strict_read_check(u64 _size, u64 type_size) const { - if (usz pos0 = pos(), size0 = size(); pos0 >= size0 || (size0 - pos0) / type_size < _size) + if (usz pos0 = pos(), size0 = size(); (pos0 >= size0 ? 0 : (size0 - pos0)) / type_size < _size) { fs::g_tls_error = fs::error::inval; return false; diff --git a/Utilities/File.h b/Utilities/File.h index adc8c4a572..f30a139e6c 100644 --- a/Utilities/File.h +++ b/Utilities/File.h @@ -205,8 +205,6 @@ namespace fs { std::unique_ptr m_file{}; - bool strict_read_check(u64 size, u64 type_size) const; - public: // Default constructor file() = default; @@ -281,6 +279,9 @@ namespace fs return m_file->sync(); } + // Check if the handle is capable of reading (size * type_size) of bytes at the time of calling + bool strict_read_check(u64 size, u64 type_size = 1) const; + // Read the data from the file and return the amount of data written in buffer u64 read(void* buffer, u64 count, u32 line = __builtin_LINE(), @@ -337,8 +338,8 @@ namespace fs } // Write std::basic_string unconditionally - template - std::enable_if_t && !std::is_pointer_v, const file&> write(const std::basic_string& str, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + const file& write(const std::basic_string& str, u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), const char* file = __builtin_FILE(), @@ -349,8 +350,8 @@ namespace fs } // Write POD unconditionally - template - std::enable_if_t && !std::is_pointer_v, const file&> write(const T& data, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + const file& write(const T& data, pod_tag_t = pod_tag, u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), @@ -362,8 +363,8 @@ namespace fs } // Write POD std::vector unconditionally - template - std::enable_if_t && !std::is_pointer_v, const file&> write(const std::vector& vec, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + const file& write(const std::vector& vec, u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), const char* file = __builtin_FILE(), @@ -373,84 +374,69 @@ namespace fs return *this; } - // Read std::basic_string, size must be set by resize() method - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::basic_string& str, - const char* file = __builtin_FILE(), - const char* func = __builtin_FUNCTION(), - u32 line = __builtin_LINE(), - u32 col = __builtin_COLUMN()) const - { - return read(&str[0], str.size() * sizeof(T), line, col, file, func) == str.size() * sizeof(T); - } - // Read std::basic_string - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::basic_string& str, usz _size, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + bool read(std::basic_string& str, usz _size = umax, const char* file = __builtin_FILE(), const char* func = __builtin_FUNCTION(), u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN()) const { if (!m_file) xnull({line, col, file, func}); - if (!_size) return true; - if constexpr (IsStrict) + if (_size != umax) { - // If _size arg is too high std::bad_alloc may happen in resize and then we cannot error check - if (!strict_read_check(_size, sizeof(T))) return false; + // If _size arg is too high std::bad_alloc may happen during resize and then we cannot error check + if ((_size >= 0x10'0000 / sizeof(T)) && !strict_read_check(_size, sizeof(T))) + { + return false; + } + + str.resize(_size); } - str.resize(_size); - return read(str.data(), sizeof(T) * _size, line, col, file, func) == sizeof(T) * _size; + return read(str.data(), sizeof(T) * str.size(), line, col, file, func) == sizeof(T) * str.size(); } // Read POD, sizeof(T) is used - template - std::enable_if_t && !std::is_pointer_v, bool> read(T& data, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + bool read(T& data, pod_tag_t = pod_tag, u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), const char* file = __builtin_FILE(), const char* func = __builtin_FUNCTION()) const { - return read(&data, sizeof(T), line, col, file, func) == sizeof(T); - } - - // Read POD std::vector, size must be set by resize() method - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::vector& vec, - const char* file = __builtin_FILE(), - const char* func = __builtin_FUNCTION(), - u32 line = __builtin_LINE(), - u32 col = __builtin_COLUMN()) const - { - return read(vec.data(), sizeof(T) * vec.size(), line, col, file, func) == sizeof(T) * vec.size(); + return read(std::addressof(data), sizeof(T), line, col, file, func) == sizeof(T); } // Read POD std::vector - template - std::enable_if_t && !std::is_pointer_v, bool> read(std::vector& vec, usz _size, + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + bool read(std::vector& vec, usz _size = umax, const char* file = __builtin_FILE(), const char* func = __builtin_FUNCTION(), u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN()) const { if (!m_file) xnull({line, col, file, func}); - if (!_size) return true; - if constexpr (IsStrict) + if (_size != umax) { - if (!strict_read_check(_size, sizeof(T))) return false; + // If _size arg is too high std::bad_alloc may happen during resize and then we cannot error check + if ((_size >= 0x10'0000 / sizeof(T)) && !strict_read_check(_size, sizeof(T))) + { + return false; + } + + vec.resize(_size); } - vec.resize(_size); - return read(vec.data(), sizeof(T) * _size, line, col, file, func) == sizeof(T) * _size; + return read(vec.data(), sizeof(T) * vec.size(), line, col, file, func) == sizeof(T) * vec.size(); } // Read POD (experimental) - template - std::enable_if_t && !std::is_pointer_v, T> read( + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + T read( pod_tag_t = pod_tag, u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), @@ -472,13 +458,13 @@ namespace fs { std::basic_string result; result.resize(size() / sizeof(T)); - if (seek(0), !read(result, file, func, line, col)) xfail({line, col, file, func}); + if (seek(0), !read(result, result.size(), file, func, line, col)) xfail({line, col, file, func}); return result; } // Read full file to std::vector - template - std::enable_if_t && !std::is_pointer_v, std::vector> to_vector( + template requires (std::is_trivially_copyable_v && !std::is_pointer_v) + std::vector to_vector( u32 line = __builtin_LINE(), u32 col = __builtin_COLUMN(), const char* file = __builtin_FILE(), @@ -486,7 +472,7 @@ namespace fs { std::vector result; result.resize(size() / sizeof(T)); - if (seek(0), !read(result, file, func, line, col)) xfail({line, col, file, func}); + if (seek(0), !read(result, result.size(), file, func, line, col)) xfail({line, col, file, func}); return result; } diff --git a/rpcs3/Emu/system_utils.cpp b/rpcs3/Emu/system_utils.cpp index 9db6b98e87..a528c6ff14 100644 --- a/rpcs3/Emu/system_utils.cpp +++ b/rpcs3/Emu/system_utils.cpp @@ -226,7 +226,7 @@ namespace rpcs3::utils // Read null-terminated string dec_file.seek(0x10); - dec_file.read(edat_content_id, 0x30); + dec_file.read(edat_content_id, 0x30); edat_content_id.resize(std::min(0x30, edat_content_id.find_first_of('\0'))); if (edat_content_id != content_id) { diff --git a/rpcs3/Loader/ELF.h b/rpcs3/Loader/ELF.h index 19dcc7c3d8..064dd05c73 100644 --- a/rpcs3/Loader/ELF.h +++ b/rpcs3/Loader/ELF.h @@ -242,14 +242,14 @@ public: if (!(opts & elf_opt::no_programs)) { stream.seek(offset + header.e_phoff); - if (!stream.read(_phdrs, header.e_phnum)) + if (!stream.read(_phdrs, header.e_phnum)) return set_error(elf_error::stream_phdrs); } if (!(opts & elf_opt::no_sections)) { stream.seek(offset + header.e_shoff); - if (!stream.read(shdrs, header.e_shnum)) + if (!stream.read(shdrs, header.e_shnum)) return set_error(elf_error::stream_shdrs); } @@ -264,7 +264,7 @@ public: if (!(opts & elf_opt::no_data)) { stream.seek(offset + hdr.p_offset); - if (!stream.read(progs.back().bin, hdr.p_filesz)) + if (!stream.read(progs.back().bin, hdr.p_filesz)) return set_error(elf_error::stream_data); } } diff --git a/rpcs3/Loader/PSF.cpp b/rpcs3/Loader/PSF.cpp index 803f4f68bb..4ede7f730d 100644 --- a/rpcs3/Loader/PSF.cpp +++ b/rpcs3/Loader/PSF.cpp @@ -189,12 +189,12 @@ namespace psf // Get indices std::vector indices; - PSF_CHECK(stream.read(indices, header.entries_num), corrupt); + PSF_CHECK(stream.read(indices, header.entries_num), corrupt); // Get keys std::string keys; PSF_CHECK(stream.seek(header.off_key_table) == header.off_key_table, corrupt); - PSF_CHECK(stream.read(keys, header.off_data_table - header.off_key_table), corrupt); + PSF_CHECK(stream.read(keys, header.off_data_table - header.off_key_table), corrupt); // Load entries for (u32 i = 0; i < header.entries_num; ++i) @@ -227,7 +227,7 @@ namespace psf { // String/array data std::string value; - PSF_CHECK(stream.read(value, indices[i].param_len), corrupt); + PSF_CHECK(stream.read(value, indices[i].param_len), corrupt); if (indices[i].param_fmt == format::string) { diff --git a/rpcs3/Loader/PUP.cpp b/rpcs3/Loader/PUP.cpp index 65af45ef01..38bfc827d5 100644 --- a/rpcs3/Loader/PUP.cpp +++ b/rpcs3/Loader/PUP.cpp @@ -46,7 +46,7 @@ pup_object::pup_object(fs::file&& file) : m_file(std::move(file)) return; } - if (!m_file.read(m_file_tbl, m_header.file_count) || !m_file.read(m_hash_tbl, m_header.file_count)) + if (!m_file.read(m_file_tbl, m_header.file_count) || !m_file.read(m_hash_tbl, m_header.file_count)) { m_error = pup_error::header_file_count; return; diff --git a/rpcs3/Loader/TROPUSR.cpp b/rpcs3/Loader/TROPUSR.cpp index df2c47445e..14e34deb7f 100644 --- a/rpcs3/Loader/TROPUSR.cpp +++ b/rpcs3/Loader/TROPUSR.cpp @@ -80,7 +80,7 @@ bool TROPUSRLoader::LoadTableHeaders() m_file.seek(0x30); m_tableHeaders.clear(); - if (!m_file.read(m_tableHeaders, m_header.tables_count)) + if (!m_file.read(m_tableHeaders, m_header.tables_count)) { return false; } @@ -103,7 +103,7 @@ bool TROPUSRLoader::LoadTables() { m_table4.clear(); - if (!m_file.read(m_table4, tableHeader.entries_count)) + if (!m_file.read(m_table4, tableHeader.entries_count)) return false; } @@ -111,7 +111,7 @@ bool TROPUSRLoader::LoadTables() { m_table6.clear(); - if (!m_file.read(m_table6, tableHeader.entries_count)) + if (!m_file.read(m_table6, tableHeader.entries_count)) return false; } diff --git a/rpcs3/Loader/TRP.cpp b/rpcs3/Loader/TRP.cpp index 29f80241d4..c462b9a922 100644 --- a/rpcs3/Loader/TRP.cpp +++ b/rpcs3/Loader/TRP.cpp @@ -40,7 +40,7 @@ bool TRPLoader::Install(const std::string& dest, bool /*show*/) { trp_f.seek(entry.offset); - if (!trp_f.read(buffer, entry.size)) + if (!trp_f.read(buffer, entry.size)) { trp_log.error("Failed to read TRPEntry at: offset=0x%x, size=0x%x", entry.offset, entry.size); continue; // ??? @@ -106,7 +106,7 @@ bool TRPLoader::LoadHeader(bool show) std::vector file_contents; trp_f.seek(0); - if (!trp_f.read(file_contents, m_header.trp_file_size)) + if (!trp_f.read(file_contents, m_header.trp_file_size)) { trp_log.notice("Failed verifying checksum"); } @@ -127,7 +127,7 @@ bool TRPLoader::LoadHeader(bool show) m_entries.clear(); - if (!trp_f.read(m_entries, m_header.trp_files_count)) + if (!trp_f.read(m_entries, m_header.trp_files_count)) { return false; }