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.
This commit is contained in:
Henrik Rydgård committed 2026-08-11 08:54:16 +02:00
1 parent 846fcf3e2c
commit 1671814be1
1 file changed
+16 -7
+16 -7
View File
@@ -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<std::mutex> 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<std::mutex> 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
}