From 1671814be137b26d89c92e61d85163afb74adbe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 11 Aug 2026 01:32:17 +0200 Subject: [PATCH] LocalFileLoader: report 0, not a huge bogus count, on read failure ReadAt()'s contract is to return the number of bytes/units actually read. On every platform branch, an OS-level read failure (ReadFile returning FALSE, or pread/read returning -1) was fed straight into a division by `bytes` without checking for it first: - Windows explicitly returned (size_t)-1. - Elsewhere, the signed -1 from pread/read was implicitly converted to size_t (via the usual arithmetic conversions with the unsigned `bytes`) before the division, producing a huge bogus count instead of a small one. Every caller in the caching chain (CachingFileLoader, RamCachingFileLoader, RetryingFileLoader, ZipFileLoader's libzip source callback) loops on "did we get at least what we asked for", which a huge return value trivially satisfies - so a local I/O error (removable media ejected, a content-URI permission problem mid-read, etc.) would be reported as a fully successful read of whatever uninitialized memory happened to be in the destination buffer. --- Core/FileLoaders/LocalFileLoader.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/Core/FileLoaders/LocalFileLoader.cpp b/Core/FileLoaders/LocalFileLoader.cpp index 921675a1c8..f359e42c84 100644 --- a/Core/FileLoaders/LocalFileLoader.cpp +++ b/Core/FileLoaders/LocalFileLoader.cpp @@ -192,33 +192,42 @@ size_t LocalFileLoader::ReadAt(s64 absolutePos, size_t bytes, size_t count, void // Toolchain has no fancy IO API. We must lock. std::lock_guard guard(readLock_); lseek(fd_, absolutePos, SEEK_SET); - return read(fd_, data, bytes * count) / bytes; + // read() returns -1 on error, not a short count. Dividing that (implicitly + // converted to a huge size_t) by bytes would otherwise report a huge bogus + // success instead of a failure, so callers would trust unwritten data. + ssize_t retval = read(fd_, data, bytes * count); + return retval < 0 ? 0 : (size_t)retval / bytes; #elif PPSSPP_PLATFORM(ANDROID) // pread64 doesn't appear to actually be 64-bit safe, though such ISOs are uncommon. See #10862. if (absolutePos <= 0x7FFFFFFF) { #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS < 64 - return pread64(fd_, data, bytes * count, absolutePos) / bytes; + ssize_t retval = pread64(fd_, data, bytes * count, absolutePos); #else - return pread(fd_, data, bytes * count, absolutePos) / bytes; + ssize_t retval = pread(fd_, data, bytes * count, absolutePos); #endif + return retval < 0 ? 0 : (size_t)retval / bytes; } else { // Since pread64 doesn't change the file offset, it should be safe to avoid the lock in the common case. std::lock_guard guard(readLock_); lseek64(fd_, absolutePos, SEEK_SET); - return read(fd_, data, bytes * count) / bytes; + ssize_t retval = read(fd_, data, bytes * count); + return retval < 0 ? 0 : (size_t)retval / bytes; } #elif !defined(_WIN32) #if defined(_FILE_OFFSET_BITS) && _FILE_OFFSET_BITS < 64 - return pread64(fd_, data, bytes * count, absolutePos) / bytes; + ssize_t retval = pread64(fd_, data, bytes * count, absolutePos); #else - return pread(fd_, data, bytes * count, absolutePos) / bytes; + ssize_t retval = pread(fd_, data, bytes * count, absolutePos); #endif + return retval < 0 ? 0 : (size_t)retval / bytes; #else DWORD read = -1; OVERLAPPED offset = { 0 }; offset.Offset = (DWORD)(absolutePos & 0xffffffff); offset.OffsetHigh = (DWORD)((absolutePos & 0xffffffff00000000) >> 32); auto result = ReadFile(handle_, data, (DWORD)(bytes * count), &read, &offset); - return result == TRUE ? (size_t)read / bytes : -1; + // On failure, report 0 bytes read rather than (size_t)-1 - callers treat the + // return value as a byte/unit count, not an error sentinel. + return result == TRUE ? (size_t)read / bytes : 0; #endif }