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;