From 51ec0459475c7dde6d0b454827b6d637e7eebb77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Tue, 8 Sep 2026 12:14:46 -0600 Subject: [PATCH] Don't drop an ISO's trailing partial sector Not every disc image is a whole number of 2048-byte sectors - tools that build pre-patched ISOs write images that stop partway through their last one, with a file legitimately ending there. Two things then conspired to lose that tail. FileBlockDevice::GetNumBlocks() rounds down, so the partial sector isn't counted, and the file size clamp in ISOFileSystem measured what the image holds in whole blocks. A file running to the last byte of such an image got clamped short - by up to a sector - before anything read it. FileBlockDevice::ReadBlock() then returned false for a short read of that sector, and ISOFileSystem::ReadFile substitutes an all-zero sector when a read fails, so even the bytes that were there came back as zeroes. Measure the clamp in bytes via GetUncompressedSize() instead of blocks, and treat a short read at the end of the image as a success with the rest of the sector zeroed. GetUncompressedSize() defaults to the block-based value and is only overridden by FileBlockDevice, so nothing else changes behaviour. Also report why a module was rejected. "Failed to load module" named the file and nothing else, and the truncation check logged only the byte count, which points at the executable when the real cause is that the loader was handed fewer bytes than the file has. ElfReader now keeps the reason for a failed LoadInto, __KernelLoadELFFromPtr puts it in the error string that reaches the user, and both messages say which header table overran and by how much. Co-Authored-By: Claude Opus 5 --- Core/ELF/ElfReader.cpp | 14 ++++++++++++-- Core/ELF/ElfReader.h | 8 ++++++++ Core/FileSystems/BlockDevices.cpp | 12 +++++++++++- Core/FileSystems/ISOFileSystem.cpp | 11 ++++++++--- Core/HLE/sceKernelModule.cpp | 11 +++++++++-- 5 files changed, 48 insertions(+), 8 deletions(-) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 8ed7d8a082..1bf565e11b 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -544,8 +544,18 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { if (header->e_ident[EI_DATA] != ELFDATA2LSB) return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; - if (size_ < header->e_phoff + sizeof(Elf32_Phdr) * GetNumSegments() || size_ < header->e_shoff + sizeof(Elf32_Shdr) * GetNumSections()) { - ERROR_LOG(Log::Loader, "Truncated ELF, %d bytes with %d sections and %d segments", (int)size_, GetNumSections(), GetNumSegments()); + const size_t phdrEnd = header->e_phoff + sizeof(Elf32_Phdr) * GetNumSegments(); + const size_t shdrEnd = header->e_shoff + sizeof(Elf32_Shdr) * GetNumSections(); + if (size_ < phdrEnd || size_ < shdrEnd) { + // Say which table runs off the end and by how much. "Truncated ELF" on its own sends you + // looking at the executable, when the usual cause is that we were handed fewer bytes than + // the file really has - a short read, or a size clamped somewhere upstream. + const char *which = size_ < phdrEnd ? "program header table" : "section header table"; + const size_t needed = size_ < phdrEnd ? phdrEnd : shdrEnd; + loadError_ = StringFromFormat( + "Truncated ELF: %s ends at %d but only %d bytes are available (%d short), %d sections, %d segments", + which, (int)needed, (int)size_, (int)(needed - size_), GetNumSections(), GetNumSegments()); + ERROR_LOG(Log::Loader, "%s", loadError_.c_str()); // Probably not the right error code. return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; } diff --git a/Core/ELF/ElfReader.h b/Core/ELF/ElfReader.h index 20424012e0..da369c86c0 100644 --- a/Core/ELF/ElfReader.h +++ b/Core/ELF/ElfReader.h @@ -174,6 +174,13 @@ public: std::vector GetCodeSections() const; int LoadInto(u32 vaddr, bool fromTop); + + // Set when LoadInto() rejects the image, so the caller can put the reason in the error it + // shows instead of only an error code. Untranslated - this is for developers and bug reports. + const std::string &LoadError() const { + return loadError_; + } + bool LoadSymbols(); bool LoadRelocations(const Elf32_Rel *rels, int numRelocs); void LoadRelocations2(int rel_seg); @@ -196,6 +203,7 @@ private: std::vector segmentVAddr; size_t size_ = 0; u32 firstSegAlign = 0; + std::string loadError_; }; // Homebrew usually ships the unstripped ELF it was built from next to the EBOOT (app.elf beside diff --git a/Core/FileSystems/BlockDevices.cpp b/Core/FileSystems/BlockDevices.cpp index e6eb46554a..b9d4729140 100644 --- a/Core/FileSystems/BlockDevices.cpp +++ b/Core/FileSystems/BlockDevices.cpp @@ -306,8 +306,18 @@ FileBlockDevice::~FileBlockDevice() {} bool FileBlockDevice::ReadBlock(int blockNumber, u8 *outPtr, bool uncached) { FileLoader::Flags flags = uncached ? FileLoader::Flags::HINT_UNCACHED : FileLoader::Flags::NONE; - size_t retval = fileLoader_->ReadAt((u64)blockNumber * (u64)GetBlockSize(), 1, 2048, outPtr, flags); + const u64 offset = (u64)blockNumber * (u64)GetBlockSize(); + size_t retval = fileLoader_->ReadAt(offset, 1, 2048, outPtr, flags); if (retval != 2048) { + // Not every image is a whole number of sectors. Tools that build pre-patched ISOs do write + // images that stop in the middle of their last sector, with a file legitimately ending + // there. The bytes that are present are real, so zero the rest of the sector and report + // success. Failing instead loses them: callers substitute an all-zero sector, which + // quietly corrupts whatever was in that tail. + if (retval > 0 && offset < filesize_) { + memset(outPtr + retval, 0, 2048 - retval); + return true; + } DEBUG_LOG(Log::FileSystem, "Could not read 2048 byte block, at block offset %d. Only got %d bytes", blockNumber, (int)retval); return false; } diff --git a/Core/FileSystems/ISOFileSystem.cpp b/Core/FileSystems/ISOFileSystem.cpp index b01837d058..b0ccb9ec22 100644 --- a/Core/FileSystems/ISOFileSystem.cpp +++ b/Core/FileSystems/ISOFileSystem.cpp @@ -307,10 +307,15 @@ void ISOFileSystem::ReadDirectory(TreeEntry *root) const { // drop the entry - truncated ISOs are common and used to work with just the warning // above, and dropping EBOOT.BIN would turn that into an unbootable game. For a sane // file this is a no-op, since the extent always fits in its sectors. + // Measured in bytes, not whole sectors: an image whose length isn't a multiple of the + // sector size still contains its final partial sector, and a file is allowed to end + // there. Counting blocks discards that tail, which clamped real files short - a dump + // with EBOOT.BIN running to the last byte of the image lost the end of it, and the ELF + // section headers that live there went with it. if (isFile) { - const u64 numBlocks = blockDevice->GetNumBlocks(); - const u64 firstSector = dir.firstDataSector; - const s64 availableBytes = firstSector >= numBlocks ? 0 : (s64)((numBlocks - firstSector) * (u64)sectorSize); + const u64 imageBytes = blockDevice->GetUncompressedSize(); + const u64 firstByte = (u64)dir.firstDataSector * (u64)sectorSize; + const s64 availableBytes = firstByte >= imageBytes ? 0 : (s64)(imageBytes - firstByte); if (entry->size > availableBytes) { entry->size = availableBytes; } diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index 8abad4708b..c55352c3f1 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -1468,7 +1468,13 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load int result = reader.LoadInto(loadAddress, fromTop); if (result != SCE_KERNEL_ERROR_OK) { - ERROR_LOG(Log::sceModule, "LoadInto failed with error %08x",result); + // Carry the reader's reason up, so what the user is shown says more than an error code. + if (!reader.LoadError().empty()) { + *error_string = reader.LoadError(); + } else { + *error_string = StringFromFormat("ELF load failed (%08x)", result); + } + ERROR_LOG(Log::sceModule, "LoadInto failed with error %08x: %s", result, error_string->c_str()); delete [] newptr; module->Cleanup(); kernelObjects.Destroy(module->GetUID()); @@ -2092,7 +2098,8 @@ static bool __KernelLoadExecFromPtr(MIPSState * mips, const u8 *data, size_t siz module->Cleanup(); kernelObjects.Destroy(module->GetUID()); } - ERROR_LOG(Log::Loader, "Failed to load module %s", filename); + ERROR_LOG(Log::Loader, "Failed to load module %s (%d bytes): %s", filename, (int)size, + error_string->empty() ? "no reason given" : error_string->c_str()); *error_string = "Failed to load executable: " + *error_string; delete[] param_argp; delete[] param_key;