From 7e79334c18cda217bb68f7774bce6bbbdd533bc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:53:48 +0200 Subject: [PATCH 01/11] ELF: bound the segment table, and reject ELFs with no loadable segment segmentVAddr is a 32-entry array, but LoadInto filled it from e_phnum, which is a u16 - so an ELF declaring 65535 program headers wrote 65535 u32s into it, straight through the rest of the ElfReader object. The only check standing in front of that verified the program headers fit in the file, which a ~2MB crafted PRX satisfies. Rejected up front now: LoadRelocations already ignores segment numbers at or past the array size, so a module with more than that couldn't be relocated correctly anyway. Real modules have a handful - PSP_Header::nsegments is a u8 and no more than 4 are ever used. Second one from the same loop: with no PT_LOAD segment at all, totalStart stayed 0xFFFFFFFF and totalEnd 0, so totalSize came out as 1 (0 - 0xFFFFFFFF) and the loader went on to allocate a 1-byte block at 0xFFFFFFFF. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ElfReader.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 770bc7f155..91fbb39cbb 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -438,6 +438,14 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; } + // e_phnum is a u16, but we can only record the load address of ARRAY_SIZE(segmentVAddr) segments, + // and the relocation code can't refer to segments beyond that either (see LoadRelocations). Real + // PSP modules have a handful - PSP_Header::nsegments is a u8 and no more than 4 are ever used. + if (GetNumSegments() > (int)ARRAY_SIZE(segmentVAddr)) { + ERROR_LOG(Log::Loader, "ELF has %d segments, we support at most %d", GetNumSegments(), (int)ARRAY_SIZE(segmentVAddr)); + return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; + } + // e_ident[EI_VERSION] is ignored // Should we relocate? @@ -467,9 +475,11 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { entryPoint = header->e_entry; u32 totalStart = 0xFFFFFFFF; u32 totalEnd = 0; + int numLoadSegments = 0; for (int i = 0; i < header->e_phnum; i++) { const Elf32_Phdr *p = &segments[i]; if (p->p_type == PT_LOAD) { + numLoadSegments++; if (p->p_vaddr < totalStart) { totalStart = p->p_vaddr; firstSegAlign = p->p_align; @@ -478,6 +488,12 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { totalEnd = p->p_vaddr + p->p_memsz; } } + // Without this, totalStart stays 0xFFFFFFFF and totalEnd 0, so totalSize would come out as 1 + // and we'd go on to allocate at 0xFFFFFFFF. + if (numLoadSegments == 0) { + ERROR_LOG(Log::Loader, "ELF has no loadable segments"); + return SCE_KERNEL_ERROR_MEMBLOCK_ALLOC_FAILED; + } totalSize = totalEnd - totalStart; // If a load address is specified that's in regular RAM, override kernel module status From 8afab6e3813e79e2b0e669a95251d85812970652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:54:53 +0200 Subject: [PATCH 02/11] ELF: validate st_shndx before using it to index sectionAddrs st_shndx is a u16 that can hold a reserved value instead of a section number, and SHN_ABS (0xFFF1) is common in real symbol tables - linker-script constants like _gp end up there. LoadSymbols fed it straight to sectionAddrs, which has GetNumSections() entries, so those symbols read a quarter of a megabyte past the allocation and added whatever they found to the symbol's address. Unlike the other bounds problems around here this one doesn't need a malformed file; any ordinary ELF with an absolute symbol hits it. Symbols that are undefined, absolute or common now get skipped instead - there's nothing of ours to relocate them against - and a section number that's in range for neither is skipped with a warning. Also bail out if LoadInto hasn't run, since that's what fills in sectionAddrs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ElfReader.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 91fbb39cbb..a2c62d0f9a 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -808,6 +808,11 @@ bool ElfReader::LoadSymbols() ERROR_LOG(Log::Loader, "Symbols truncated - ignoring"); return false; } + // Relocating a symbol needs the section addresses LoadInto computed. + if (bRelocate && !sectionAddrs) { + ERROR_LOG(Log::Loader, "LoadSymbols called before LoadInto - ignoring"); + return false; + } for (int sym = 0; sym= size_) continue; - if (bRelocate) + if (bRelocate) { + // st_shndx is a u16 that can hold reserved values rather than a section number - + // SHN_ABS (0xFFF1) in particular is common and means the value is already final. + // Indexing sectionAddrs (which has GetNumSections() entries) with one of those read + // far out of bounds and added whatever it found to the symbol's address. + if (sectionIndex == SHN_UNDEF || sectionIndex >= SHN_LORESERVE) { + // Undefined, absolute or common - nothing of ours to relocate against. + continue; + } + if (sectionIndex >= GetNumSections()) { + WARN_LOG(Log::Loader, "Symbol '%s' refers to bad section %d, skipping", name, sectionIndex); + continue; + } value += sectionAddrs[sectionIndex]; + } switch (type) { From 916863e2eb1f5af80f710ff67c1ae49337f19de5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:55:37 +0200 Subject: [PATCH 03/11] ELF: bounds-check sh_info before indexing the section table Both relocation branches took the section to modify straight from the file's sh_info and only checked it wasn't negative, so a value like 1000 in a ten-section file read past the end of the section table and decided what to relocate based on whatever was there. sh_info is a u32, so ">= 0" only rejected the half of the range above INT_MAX. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ElfReader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index a2c62d0f9a..4d54d53e57 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -644,7 +644,7 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { { //We have a relocation table! int sectionToModify = s->sh_info; - if (sectionToModify >= 0) + if (sectionToModify >= 0 && sectionToModify < GetNumSections()) { if (!(sections[sectionToModify].sh_flags & SHF_ALLOC)) { @@ -679,7 +679,7 @@ int ElfReader::LoadInto(u32 loadAddress, bool fromTop) { { //We have a relocation table! int sectionToModify = s->sh_info; - if (sectionToModify >= 0) + if (sectionToModify >= 0 && sectionToModify < GetNumSections()) { if (!(sections[sectionToModify].sh_flags & SHF_ALLOC)) { From 077205da9bdef3a0e5aaec90a0a6a2ae0aa9c707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:57:17 +0200 Subject: [PATCH 04/11] ParamSFO: stop WriteSFO writing past the space an entry reserved Each entry gets param_max_len bytes in the data table, and that's what the buffer is sized from - but nothing clamped what got written into it. For VT_UTF8 the length written was s_value.size()+1, and then a terminator was stored at data_ptr[param_len], one byte beyond that again. The memcpy already copies the terminator (param_len counts it), so that store was both redundant and always out of range. It doesn't take a malformed file to hit: several callers pass the string's own length as max_size - see PSPLoaders.cpp's TITLE, DISC_ID and DISC_VERSION - so the entry overran by two bytes every time, and a 128-character SAVEDATA_TITLE in a 128-byte slot wrote its terminator into the next entry's data. VT_UTF8_SPE had the same missing clamp without the off-by-one. Both are clamped now and log when they truncate, and a negative max_size no longer subtracts from the computed buffer size. Bytes written are unchanged for values that do fit, which is every normal case - the terminator now comes from the zero-fill instead of an explicit store - so this doesn't change any savedata the emulator produces. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ParamSFO.cpp | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/Core/ELF/ParamSFO.cpp b/Core/ELF/ParamSFO.cpp index 0e605c6be5..f3586b6b85 100644 --- a/Core/ELF/ParamSFO.cpp +++ b/Core/ELF/ParamSFO.cpp @@ -15,6 +15,7 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. +#include #include #include @@ -250,6 +251,12 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, size_t size, const char *dat return -1; } +// How many bytes an entry gets in the data table. Everything written for it has to fit in here - +// the size loop below and the fill loop after it both go through this, so they can't disagree. +static u32 ReservedSize(const ParamSFOData::ValueData &value) { + return (u32)std::max(0, value.max_size); +} + void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { size_t total_size = 0; size_t key_size = 0; @@ -266,7 +273,7 @@ void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { for (const auto &[k, v] : values) { key_size += k.size() + 1; - data_size += v.max_size; + data_size += ReservedSize(v); header.index_table_entries++; } @@ -299,35 +306,49 @@ void ParamSFOData::WriteSFO(u8 **paramsfo, size_t *size) const { index_ptr->key_table_offset = offset; offset = (u16)(data_ptr - (data+header.data_table_start)); index_ptr->data_table_offset = offset; - index_ptr->param_max_len = v.max_size; + const u32 reserved = ReservedSize(v); + index_ptr->param_max_len = reserved; if (v.type == VT_INT) { index_ptr->param_fmt = 0x0404; - index_ptr->param_len = 4; + index_ptr->param_len = std::min(4u, reserved); - *(s32_le *)data_ptr = v.i_value; + if (reserved >= 4) + *(s32_le *)data_ptr = v.i_value; + else + WARN_LOG(Log::Loader, "SFO key '%s' is an int but only reserves %d bytes, dropping", k.c_str(), (int)reserved); } else if (v.type == VT_UTF8_SPE) { index_ptr->param_fmt = 0x0004; - index_ptr->param_len = (u32)v.u_value.size(); + // Raw data, no terminator, but it still has to fit in what the entry reserved. + const u32 len = std::min((u32)v.u_value.size(), reserved); + if (len != v.u_value.size()) + WARN_LOG(Log::Loader, "SFO key '%s': %d bytes of data truncated to %d", k.c_str(), (int)v.u_value.size(), (int)len); + index_ptr->param_len = len; - memset(data_ptr, 0, index_ptr->param_max_len); - memcpy(data_ptr, v.u_value.data(), index_ptr->param_len); + memset(data_ptr, 0, reserved); + memcpy(data_ptr, v.u_value.data(), len); } else if (v.type == VT_UTF8) { index_ptr->param_fmt = 0x0204; - index_ptr->param_len = (u32)v.s_value.size()+1; + // param_len counts the NUL terminator, so the string itself gets reserved - 1 bytes. + // Several callers pass the string's own length as max_size (see PSPLoaders.cpp), which + // used to overrun the entry by the terminator plus one more from the stray write below. + const u32 len = std::min((u32)v.s_value.size(), reserved ? reserved - 1 : 0); + if (len != v.s_value.size()) + WARN_LOG(Log::Loader, "SFO key '%s': string of %d chars truncated to %d", k.c_str(), (int)v.s_value.size(), (int)len); + index_ptr->param_len = reserved ? len + 1 : 0; - memcpy(data_ptr,v.s_value.c_str(),index_ptr->param_len); - data_ptr[index_ptr->param_len] = 0; + memset(data_ptr, 0, reserved); // Also supplies the terminator. + memcpy(data_ptr, v.s_value.data(), len); } memcpy(key_ptr,k.c_str(),k.size()); key_ptr[k.size()] = 0; - data_ptr += index_ptr->param_max_len; + data_ptr += reserved; key_ptr += k.size() + 1; index_ptr++; From 1a4a1db1b62044e19b44214f082c3b8b3392ed7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:57:53 +0200 Subject: [PATCH 05/11] PBPReader: don't write the header through a const-stripping cast header_ was declared const and value-initialized, and then the constructor read the file into it via (u8 *)&header_. The C-style cast makes that compile, but modifying a const object is undefined - the compiler is entitled to keep assuming header_ still holds the zeroes it was initialized with, and fold reads of it accordingly. It happens to work today; there's no reason to keep relying on that. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/PBPReader.cpp | 2 +- Core/ELF/PBPReader.h | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Core/ELF/PBPReader.cpp b/Core/ELF/PBPReader.cpp index baca705045..0a4868688e 100644 --- a/Core/ELF/PBPReader.cpp +++ b/Core/ELF/PBPReader.cpp @@ -29,7 +29,7 @@ PBPReader::PBPReader(FileLoader *fileLoader) { } fileSize_ = (size_t)fileLoader->FileSize(); - if (fileLoader->ReadAt(0, sizeof(header_), (u8 *)&header_) != sizeof(header_)) { + if (fileLoader->ReadAt(0, sizeof(header_), &header_) != sizeof(header_)) { ERROR_LOG(Log::Loader, "PBP is too small to be valid: %s", fileLoader->GetPath().c_str()); return; } diff --git a/Core/ELF/PBPReader.h b/Core/ELF/PBPReader.h index 572f215ad5..97fa545180 100644 --- a/Core/ELF/PBPReader.h +++ b/Core/ELF/PBPReader.h @@ -65,6 +65,9 @@ public: private: FileLoader *file_ = nullptr; size_t fileSize_ = 0; - const PBPHeader header_{}; + // Not const: the constructor reads the file straight into this. It used to be, and was written + // through a cast that stripped the const away - which compiles, but lets the compiler assume the + // value never changes from the {} it was initialized with. + PBPHeader header_{}; bool isELF_ = false; }; From 12fa56d8421b6448496a629b11f6d62b5dcc16a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 18:59:52 +0200 Subject: [PATCH 06/11] PrxDecrypter: require a whole header before decrypting Every decrypt type reads the tag at 0xD0, the compressed size at 0xB0 and key data as far as 0x150, and writes a KIRK header into outbuf at a fixed offset derived from sizeof(PSP_Header) - all without checking that either buffer is that big. A PRX declaring a tiny psp_size therefore read past the end of its input, wrote a 0xE0-byte header past the end of an equally tiny output buffer, and handed KIRK "size - offset" as an unsigned underflow. The header write sits behind the SHA-1 check, but the tag keys are compiled in and every hashed input comes from the file, so that's arithmetic rather than luck. One size check at the top of pspDecryptPRX covers all five types. The module loader needed two things to go with it. Its "maybe it just isn't encrypted" fallback checked for ELF magic at 0x150 of the *output* buffer, which on the paths where decryption bails early has nothing written to it yet - so it read uninitialized heap to decide, and then, if psp_size was under 0x150, memcpy'd a negative length. It reads the input buffer now, which is what it goes on to copy from anyway, and only when psp_size is big enough to hold what's being tested. Second, the returned size is just comp_size out of the file header, checked against the allocated buffer by a _dbg_assert_ that isn't there in release. That check is a real one now, folded into the existing sanity test next to it. pspautotests cpu 11/11. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/PrxDecrypter.cpp | 10 ++++++++++ Core/HLE/sceKernelModule.cpp | 12 ++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Core/ELF/PrxDecrypter.cpp b/Core/ELF/PrxDecrypter.cpp index edccbb2597..27ae004e44 100644 --- a/Core/ELF/PrxDecrypter.cpp +++ b/Core/ELF/PrxDecrypter.cpp @@ -1017,6 +1017,16 @@ static int pspDecryptType6(KirkState *kirk, const u8 *inbuf, u8 *outbuf, u32 siz int pspDecryptPRX(const u8 *inbuf, u8 *outbuf, u32 size, const u8 *seed) { + // Every type below reads the tag at 0xD0, the size at 0xB0 and key data as far as 0x150, and + // writes a KIRK header into outbuf at a fixed offset derived from sizeof(PSP_Header). Without + // this, a PRX declaring a tiny psp_size got read past its end, had a header written past the end + // of the (equally tiny) output buffer, and passed "size - offset" to KIRK as an unsigned + // underflow. Callers must supply at least a whole header on both sides. + if (size < sizeof(PSP_Header)) { + ERROR_LOG(Log::Loader, "PRX too small to decrypt: %d bytes, need at least %d", (int)size, (int)sizeof(PSP_Header)); + return -1; + } + KirkState kirk{}; kirk_init(&kirk); diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index da425070ea..8cdb363bf0 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -1159,15 +1159,19 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load elfSize = maxElfSize; ptr = newptr; int decryptedSize = pspDecryptPRX(in, (u8*)ptr, head->psp_size); - _dbg_assert_(decryptedSize <= (int)maxElfSize); - if (decryptedSize <= 0 && Read32(ptr + 0x150) == ELF_MAGIC) { + // If decryption got us nowhere, the PRX may simply not be encrypted - in which case the ELF + // starts right after the header. Check the source buffer, not the destination: on the paths + // where decryption bails early nothing has been written to newptr yet, so this used to read + // uninitialized heap to decide. psp_size is known to be <= the data we actually have. + if (decryptedSize <= 0 && head->psp_size >= 0x150 + sizeof(u32) && Read32(in + 0x150) == ELF_MAGIC) { decryptedSize = head->psp_size - 0x150; memcpy(newptr, in + 0x150, decryptedSize); // In this case it's definitely not compressed. Added assert below. } - // Don't accept ELFs over 24MB - nor ones with negative size, of course. - if (decryptedSize < 0 || decryptedSize > 24 * 1024 * 1024) { + // Don't accept ELFs over 24MB, ones bigger than the buffer we allocated for them - nor ones + // with negative size, of course. + if (decryptedSize < 0 || decryptedSize > 24 * 1024 * 1024 || decryptedSize > (int)maxElfSize) { *error_string = StringFromFormat("ELF/PRX corrupt, unreasonable decrypted size: %d", (u32)decryptedSize); // TODO: Might be the wrong error code. error = SCE_KERNEL_ERROR_FILEERR; From ec364e2cd1aaf209be4a63566bed7d8ac8544c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 19:01:07 +0200 Subject: [PATCH 07/11] ELF: three fixes in the HI16/LO16 relocation path addrToHiLo's verification computed (hi<<16) + lo with hi a u16, which promotes to int - so for any kernel module, loading at 0x88000000, the shift overflowed a signed int. Undefined behaviour in the one place whose whole job is to check that a relocation came out right. A HI16 that found no matching LO16 logged an error and then wrote its zero- initialized hi into the instruction anyway, blanking the immediate of a lui it had just admitted it couldn't resolve. It leaves the instruction alone now: we don't know the right value, and a zeroed lui produces a wrong address far from here rather than a failure anyone can trace back. And the candidate LO16's address was computed with the HI16's segment base rather than its own, which is exactly the mismatch the warning a few lines below exists to report - so when that warning fired, the IsValidAddress check guarding the pairing had been applied to an address from the wrong segment. pspautotests 314/314 with --graphics=software. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ElfReader.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index 4d54d53e57..de16086dd0 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -56,7 +56,9 @@ void addrToHiLo(u32 addr, u16 &hi, s16 &lo) lo = (addr & 0xFFFF); u32 naddr = addr - lo; hi = naddr>>16; - u32 test = (hi<<16) + lo; + // Note the casts: hi is a u16, so it promotes to int, and kernel modules load at 0x88000000 - + // shifting a value of 0x8800 left by 16 would overflow a signed int. + u32 test = ((u32)hi << 16) + (u32)lo; if (test != addr) { WARN_LOG_REPORT(Log::Loader, "HI16/LO16 relocation failure?"); @@ -161,7 +163,12 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { if (t_type == R_MIPS_HI16) continue; - u32 corrLoAddr = rels[t].r_offset + segmentVAddr[readwrite]; + // The candidate LO16 declares its own segment - use that rather than the HI16's, + // which is what the mismatch warning further down is there to detect. + int t_readwrite = (rels[t].r_info >> 8) & 0xff; + if (t_readwrite >= (int)ARRAY_SIZE(segmentVAddr)) + continue; + u32 corrLoAddr = rels[t].r_offset + segmentVAddr[t_readwrite]; // In MotorStorm: Arctic Edge (US), these are sometimes R_MIPS_16 (instead of LO16.) // It appears the PSP takes any relocation that is not a HI16. @@ -194,10 +201,14 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { ERROR_LOG(Log::Loader, "Bad corrLoAddr %08x", corrLoAddr); } } - if (!found) { + if (found) { + op = (op & 0xFFFF0000) | hi; + } else { + // Leave the instruction alone rather than writing hi's initial 0 into it. We + // have no idea what the right immediate is, and zeroing the lui of a lui/addiu + // pair is a guess that's wrong in a way that's hard to trace back to here. ERROR_LOG_REPORT(Log::Loader, "R_MIPS_HI16: could not find R_MIPS_LO16 (r=%d of %d, addr=%08x)", r, numRelocs, addr); } - op = (op & 0xFFFF0000) | hi; } break; From 90a178aeb8a95048bc99c4544003a16af8a3ee94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 19:02:00 +0200 Subject: [PATCH 08/11] ParamSFO: make GenerateFakeID independent of char signedness The fake disc ID homebrew gets when it has no PARAM.SFO is built from the sum of the bytes of its folder name, summed through a plain char - which is signed on x86 and unsigned on ARM. So the same homebrew folder produced one ID on Windows and a different one on Android, quietly splitting its savestates and per-game config between platforms. Sum through unsigned char, which is what the ARM builds (Android, iOS, Apple Silicon) already did. Uppercasing is now explicit and ASCII-only rather than toupper(). Passing a negative char to toupper() is undefined and trips MSVC's debug CRT assert, so a folder with a non-ASCII name could stop a debug build dead, and what it did with bytes above 0x7F otherwise depended on the locale. ASCII folder names - very nearly all of them - produce exactly the same ID as before. Non-ASCII ones change on the signed-char platforms, to what the unsigned-char ones were already generating. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ParamSFO.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Core/ELF/ParamSFO.cpp b/Core/ELF/ParamSFO.cpp index f3586b6b85..2e6a3fe1ad 100644 --- a/Core/ELF/ParamSFO.cpp +++ b/Core/ELF/ParamSFO.cpp @@ -369,14 +369,18 @@ std::string ParamSFOData::GenerateFakeID(const Path &filename) const { std::string file = path.GetFilename(); + // Deliberately byte-wise and ASCII-only. Filenames are UTF-8, and a plain char is signed on x86 + // and unsigned on ARM - so summing chars directly gave Windows and Android different IDs for the + // same non-ASCII folder name, and toupper() on a negative value trips MSVC's debug CRT. ASCII + // names, which is very nearly all of them, produce exactly the same ID as before either way. int sumOfAllLetters = 0; for (char &c : file) { - sumOfAllLetters += c; + sumOfAllLetters += (unsigned char)c; // Get rid of some garbage characters than can arise when opening content URIs. Well, I've only seen '%', but... - if (strchr("%() []", c) != nullptr) { + if (c && strchr("%() []", c) != nullptr) { c = 'X'; - } else { - c = toupper(c); + } else if (c >= 'a' && c <= 'z') { + c = c - 'a' + 'A'; } } From 4b282383b2a9519b3a27c8a83136af1284bce687 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 19:05:26 +0200 Subject: [PATCH 09/11] ELF: harden the accessors and the Rel2 relocation decoder Nothing here is known to misbehave on a real file - it's the input validation around the fixes in the preceding commits. ElfReader's constructor read e_phoff and e_shoff out of the header to find the segment and section tables, before anything had established there was a header there; LoadInto's size check only runs later. It leaves header null in that case now, and the accessors that use it cope. GetSegmentPtr didn't range-check the segment index at all, and both it and GetSectionDataPtr accepted an offset exactly at the end of the file, which addresses no bytes. GetSectionAddr and GetSectionSize took an index on trust. LoadRelocations2 got most of this commit. Its segment end came from p_filesz without checking the segment fits in the file, so the whole decode could run off the end of the buffer. Within it, the flag and type tables are indexed by bitfields out of each command word and were never checked against the table sizes (which themselves come from the file); the loop only guaranteed one byte was left before reading a two-byte command, and the branches that consume a further two or four bytes checked nothing at all; and the offset segment number - unlike the address segment number a few lines up - was used to index segmentVAddr unchecked, though it's wide enough to exceed it. The command read is byte-wise now too: how far buf has advanced depends on those file-supplied table sizes, so it isn't necessarily even. LoadSymbols only checked that a symbol name started inside the file, not that it was terminated there. Also dropped the atomic counter and the ParallelLoop.h include left over from when LoadRelocations ran in parallel. pspautotests 314/314 with --graphics=software. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/ElfReader.cpp | 72 ++++++++++++++++++++++++++++++++++++------ Core/ELF/ElfReader.h | 32 +++++++++++++------ 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/Core/ELF/ElfReader.cpp b/Core/ELF/ElfReader.cpp index de16086dd0..4efc8cda6c 100644 --- a/Core/ELF/ElfReader.cpp +++ b/Core/ELF/ElfReader.cpp @@ -15,10 +15,7 @@ // Official git repository and contact information can be found at // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. -#include - #include "Common/StringUtils.h" -#include "Common/Thread/ParallelLoop.h" #include "Common/File/DirListing.h" #include "Common/File/FileUtil.h" @@ -70,8 +67,7 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { relocOps.resize(numRelocs); DEBUG_LOG(Log::Loader, "Loading %i relocations...", numRelocs); - std::atomic numErrors; - numErrors.store(0); + int numErrors = 0; { for (int r = 0; r < numRelocs; r++) { @@ -250,7 +246,7 @@ bool ElfReader::LoadRelocations(const Elf32_Rel *rels, int numRelocs) { } if (numErrors) { - WARN_LOG(Log::Loader, "%i bad relocations found!!!", numErrors.load()); + WARN_LOG(Log::Loader, "%i bad relocations found!!!", numErrors); } return numErrors == 0; } @@ -274,8 +270,26 @@ void ElfReader::LoadRelocations2(int rel_seg) ERROR_LOG_REPORT(Log::Loader, "Rel2 segment invalid"); return; } + // GetSegmentPtr only vouches for where the segment starts - p_filesz comes from the file too. + if ((size_t)ph->p_offset + ph->p_filesz > size_) { + ERROR_LOG_REPORT(Log::Loader, "Rel2 segment extends past the end of the file"); + return; + } end = buf+ph->p_filesz; + // Everything below reads forward from buf, so check there's something there each time. All of + // these sizes and indexes come out of the file. + auto haveBytes = [&buf, &end](int n) -> bool { + if (end - buf < n) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: truncated relocation data"); + return false; + } + return true; + }; + + if (!haveBytes(4)) + return; + flag_bits = buf[2]; type_bits = buf[3]; @@ -285,22 +299,40 @@ void ElfReader::LoadRelocations2(int rel_seg) buf += 4; + // Both tables are prefixed by their own size, and are indexed by bitfields out of the command + // words below - so the tables and the indexes into them both need checking. + if (!haveBytes(1)) + return; flag_table = buf; flag_table_size = flag_table[0]; + if (!haveBytes(flag_table_size)) + return; buf += flag_table_size; + if (!haveBytes(1)) + return; type_table = buf; type_table_size = type_table[0]; + if (!haveBytes(type_table_size)) + return; buf += type_table_size; rel_base = 0; last_type = -1; while(buf>(16-flag_bits))&0xffff; + if (flag >= flag_table_size) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: flag %d out of range (table has %d)", flag, flag_table_size); + return; + } flag = flag_table[flag]; seg = (cmd<<(16-seg_bits-flag_bits))&0xffff; @@ -308,6 +340,10 @@ void ElfReader::LoadRelocations2(int rel_seg) type = ( cmd<<(16-type_bits-seg_bits-flag_bits))&0xffff; type = (type>>(16-type_bits))&0xffff; + if (type >= type_table_size) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: type %d out of range (table has %d)", type, type_table_size); + return; + } type = type_table[type]; if((flag&0x01)==0){ @@ -315,6 +351,8 @@ void ElfReader::LoadRelocations2(int rel_seg) if((flag&0x06)==0){ rel_base = cmd>>(seg_bits+flag_bits); }else if((flag&0x06)==4){ + if (!haveBytes(4)) + return; rel_base = buf[0] | (buf[1]<<8) | (buf[2]<<16) | (buf[3]<<24); buf += 4; }else{ @@ -344,17 +382,25 @@ void ElfReader::LoadRelocations2(int rel_seg) if(cmd&0x8000) rel_offset |= 0xffff0000; rel_offset >>= type_bits+seg_bits+flag_bits; + if (!haveBytes(2)) + return; rel_offset = (rel_offset<<16) | (buf[0]) | (buf[1]<<8); buf += 2; rel_base += rel_offset; }else if((flag&0x06)==0x04){ + if (!haveBytes(4)) + return; rel_base = buf[0] | (buf[1]<<8) | (buf[2]<<16) | (buf[3]<<24); buf += 4; }else{ ERROR_LOG_REPORT(Log::Loader, "Rel2: invalid relocat size flag! %x", flag); } - + // seg is seg_bits wide, which can address more segments than we can record. + if (off_seg >= (int)ARRAY_SIZE(segmentVAddr)) { + ERROR_LOG_REPORT(Log::Loader, "Rel2: bad offset segment %d", off_seg); + continue; + } rel_offset = rel_base+segmentVAddr[off_seg]; if (!Memory::IsValidAddress(rel_offset)) { ERROR_LOG_REPORT(Log::Loader, "ELF: Bad rel_offset: %08x", rel_offset); @@ -367,6 +413,8 @@ void ElfReader::LoadRelocations2(int rel_seg) if(last_type!=0x04) lo16 = 0; }else if((flag&0x38)==0x10){ + if (!haveBytes(2)) + return; lo16 = (buf[0]) | (buf[1]<<8); if(lo16&0x8000) lo16 |= 0xffff0000; @@ -815,7 +863,7 @@ bool ElfReader::LoadSymbols() u32 symtabOffset = GetSectionDataOffset(sec); int numSymbols = sections[sec].sh_size / sizeof(Elf32_Sym); - if (!stringBase || !symtab || symtabOffset + sections[sec].sh_size > size_) { + if (!stringBase || !symtab || (size_t)symtabOffset + sections[sec].sh_size > size_) { ERROR_LOG(Log::Loader, "Symbols truncated - ignoring"); return false; } @@ -835,8 +883,12 @@ bool ElfReader::LoadSymbols() int type = symtab[sym].st_info & 0xF; int sectionIndex = symtab[sym].st_shndx; int value = symtab[sym].st_value; + const size_t nameOffset = (size_t)stringOffset + symtab[sym].st_name; + if (nameOffset >= size_) + continue; const char *name = stringBase + symtab[sym].st_name; - if (stringOffset + symtab[sym].st_name >= size_) + // And make sure it's terminated inside the file, before anything strlen()s it. + if (strnlen(name, size_ - nameOffset) == size_ - nameOffset) continue; if (bRelocate) { diff --git a/Core/ELF/ElfReader.h b/Core/ELF/ElfReader.h index 132a7f8669..4d3192e7fa 100644 --- a/Core/ELF/ElfReader.h +++ b/Core/ELF/ElfReader.h @@ -51,10 +51,14 @@ public: ElfReader(const void *ptr, size_t size) { base = (const char*)ptr; base32 = (const u32 *)ptr; + size_ = size; + // Don't read the header to find the segment and section tables before we know it's there. + // LoadInto() rejects anything this small; header stays null so nothing else can use it either. + if (size < sizeof(Elf32_Ehdr)) + return; header = (const Elf32_Ehdr*)ptr; segments = (const Elf32_Phdr *)(base + header->e_phoff); sections = (const Elf32_Shdr *)(base + header->e_shoff); - size_ = size; } ~ElfReader() { @@ -66,21 +70,22 @@ public: return base32[off >> 2]; } - // Quick accessors - ElfType GetType() const { return (ElfType)(u16)(header->e_type); } - ElfMachine GetMachine() const { return (ElfMachine)(u16)(header->e_machine); } + // Quick accessors. header is null if we weren't even handed a full ELF header, see the + // constructor - so these all have to cope with that. + ElfType GetType() const { return header ? (ElfType)(u16)(header->e_type) : (ElfType)0; } + ElfMachine GetMachine() const { return header ? (ElfMachine)(u16)(header->e_machine) : (ElfMachine)0; } u32 GetEntryPoint() const { return entryPoint; } - u32 GetFlags() const { return (u32)(header->e_flags); } + u32 GetFlags() const { return header ? (u32)(header->e_flags) : 0; } - int GetNumSegments() const { return (int)(header->e_phnum); } - int GetNumSections() const { return (int)(header->e_shnum); } + int GetNumSegments() const { return header ? (int)(header->e_phnum) : 0; } + int GetNumSections() const { return header ? (int)(header->e_shnum) : 0; } const char *GetSectionName(int section) const; const u8 *GetPtr(u32 offset) const { return (const u8*)base + offset; } // Note: zero is not a valid output, means unavailable. u32 GetSectionDataOffset(int section) const { - if (section < 0 || section >= header->e_shnum) + if (section < 0 || section >= GetNumSections()) return 0; if (sections[section].sh_type == SHT_NOBITS) return 0; @@ -88,19 +93,26 @@ public: } const u8 *GetSectionDataPtr(int section) const { u32 offset = GetSectionDataOffset(section); - if (offset == 0 || offset > size_) + // Note >=: an offset exactly at the end of the file addresses no bytes at all. + if (offset == 0 || offset >= size_) return nullptr; return GetPtr(offset); } const u8 *GetSegmentPtr(int segment) const { - if (segments[segment].p_offset > size_) + if (segment < 0 || segment >= GetNumSegments()) + return nullptr; + if (segments[segment].p_offset >= size_) return nullptr; return GetPtr(segments[segment].p_offset); } u32 GetSectionAddr(SectionID section) const { + if (section < 0 || section >= GetNumSections() || !sectionAddrs) + return 0; return sectionAddrs[section]; } int GetSectionSize(SectionID section) const { + if (section < 0 || section >= GetNumSections()) + return 0; return sections[section].sh_size; } From 5020647bab7ce83f54d01b4469e875003fd128ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 19:09:42 +0200 Subject: [PATCH 10/11] PBPReader: detect ELFs by their actual magic, and sanity-check subfile sizes The "is this an ELF rather than a PBP" test compared against "\nFLE", which is neither ELF's magic (\x7fELF) nor anything else - most likely a \x7f escape that swallowed the E when it was written in 2013. Since no real file matches it, every file that wasn't a PBP was reported as an ELF and the error branch was unreachable. Compares against the real magic now, so something that's neither is reported as neither. That error also printed the 4-byte magic with %s, which isn't NUL-terminated - it's four hex bytes instead. GetSubFileSize subtracted offsets that come straight out of the file without checking they're ordered or even inside it, so a corrupt PBP produced a size from an unsigned underflow - nearly 4GB, which the callers then had to catch by size limit. It returns 0 for anything that doesn't make sense. Also &(*out)[0] on a zero-length subfile, which is UB on an empty vector. Plus one in ParamSFO: GetDataOffset mixed int and u32 for the data offset, so its bounds check ran in whichever type the promotion landed on. It's size_t throughout now, matching how ReadSFO does the same arithmetic. Booted an EBOOT.PBP to check the PBP path end to end - loads, and generates the same fake disc ID as before. pspautotests 314/314, UnitTest 55/55. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/ELF/PBPReader.cpp | 12 +++++++++--- Core/ELF/PBPReader.h | 18 ++++++++++++------ Core/ELF/ParamSFO.cpp | 10 +++++++--- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/Core/ELF/PBPReader.cpp b/Core/ELF/PBPReader.cpp index 0a4868688e..96f9819352 100644 --- a/Core/ELF/PBPReader.cpp +++ b/Core/ELF/PBPReader.cpp @@ -34,11 +34,15 @@ PBPReader::PBPReader(FileLoader *fileLoader) { return; } if (memcmp(header_.magic, "\0PBP", 4) != 0) { - if (memcmp(header_.magic, "\nFLE", 4) != 0) { + // Split string so the \x7f escape doesn't swallow the E. This used to compare against + // "\nFLE", which is neither ELF's magic nor anything else - so every file that wasn't a PBP + // was reported as an ELF, and the error branch below was unreachable. + if (memcmp(header_.magic, "\x7f" "ELF", 4) == 0) { VERBOSE_LOG(Log::Loader, "%s: File actually an ELF, not a PBP", fileLoader->GetPath().c_str()); isELF_ = true; } else { - ERROR_LOG(Log::Loader, "Magic number in %s indicated no PBP: %s", fileLoader->GetPath().c_str(), header_.magic); + ERROR_LOG(Log::Loader, "Magic number in %s indicates neither PBP nor ELF: %02x %02x %02x %02x", + fileLoader->GetPath().c_str(), (u8)header_.magic[0], (u8)header_.magic[1], (u8)header_.magic[2], (u8)header_.magic[3]); } return; } @@ -63,7 +67,9 @@ bool PBPReader::GetSubFile(PBPSubFile file, std::vector *out) const { const u32 off = header_.offsets[(int)file]; out->resize(expected); - size_t bytes = file_->ReadAt(off, expected, &(*out)[0]); + if (expected == 0) + return true; + size_t bytes = file_->ReadAt(off, expected, out->data()); if (bytes != expected) { ERROR_LOG(Log::Loader, "PBP file read truncated: %d -> %d", (int)expected, (int)bytes); if (bytes < expected) { diff --git a/Core/ELF/PBPReader.h b/Core/ELF/PBPReader.h index 97fa545180..8781aa66a7 100644 --- a/Core/ELF/PBPReader.h +++ b/Core/ELF/PBPReader.h @@ -20,6 +20,7 @@ #include +#include "Common/Common.h" #include "Common/CommonTypes.h" #include "Common/Swap.h" @@ -54,12 +55,17 @@ public: bool GetSubFileAsString(PBPSubFile file, std::string *out) const; size_t GetSubFileSize(PBPSubFile file) const { - int num = (int)file; - if (num < 7) { - return header_.offsets[file + 1] - header_.offsets[file]; - } else { - return fileSize_ - header_.offsets[file]; - } + const int num = (int)file; + if (num < 0 || num >= (int)ARRAY_SIZE(header_.offsets)) + return 0; + const u32 start = header_.offsets[num]; + // The last subfile runs to the end of the file, the rest to where the next one starts. + const u32 stop = num + 1 < (int)ARRAY_SIZE(header_.offsets) ? header_.offsets[num + 1] : (u32)fileSize_; + // These offsets come out of the file, so they aren't necessarily ordered or even inside it. + // Subtracting them blind produced a huge size from an underflow. + if (stop < start || stop > fileSize_) + return 0; + return stop - start; } private: diff --git a/Core/ELF/ParamSFO.cpp b/Core/ELF/ParamSFO.cpp index 2e6a3fe1ad..f969547c02 100644 --- a/Core/ELF/ParamSFO.cpp +++ b/Core/ELF/ParamSFO.cpp @@ -228,14 +228,18 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, size_t size, const char *dat const IndexTable *indexTables = (const IndexTable *)(paramsfo + sizeof(Header)); const u8 *key_start = paramsfo + header->key_table_start; - int data_start = header->data_table_start; + const size_t data_start = header->data_table_start; for (u32 i = 0; i < header->index_table_entries; i++) { + // In size_t throughout - these are u32s from the file, and mixing them with int meant the + // bounds check below was done in whatever type the promotion landed on. ReadSFO does the + // same arithmetic this way. size_t key_offset = header->key_table_start + indexTables[i].key_table_offset; if (key_offset >= size) continue; - if (data_start + indexTables[i].data_table_offset >= (int)size) + size_t data_offset = data_start + indexTables[i].data_table_offset; + if (data_offset >= size) continue; const char *key = (const char *)(key_start + indexTables[i].key_table_offset); @@ -244,7 +248,7 @@ int ParamSFOData::GetDataOffset(const u8 *paramsfo, size_t size, const char *dat continue; if (!strcmp(key, dataName)) { - return data_start + indexTables[i].data_table_offset; + return (int)data_offset; } } From 4c72c13a0d4812a216a2e34077239b2687e0040a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Wed, 19 Aug 2026 19:17:21 +0200 Subject: [PATCH 11/11] Module loader: clean up properly on the two failure paths that didn't __KernelLoadELFFromPtr creates its PSPModule and inserts it into loadedModules before it knows whether the file is loadable, so every failure exit has to delete the decrypt buffer, Cleanup() the module and Destroy() it. Five of the seven did. The "unreasonable decrypted size" exit and the decompression-failure exit just returned - leaking the buffer, and leaving a live kernel object with its UID stuck in loadedModules for the rest of the session. While tracing that: the fake-module path frees newptr and then runs for another sixty lines with ptr still pointing into it. Nothing reads it today - the exits below use head, which points into the original input rather than the copy - so there's no use-after-free and no double free, but that's a property of the current code rather than anything enforced. Both pointers are nulled after the delete so a future mistake there crashes instead of reading freed heap. And the function read the magic, and in the ~SCE branch a second word after it, before anything established the input was that big. The non-PBP caller guarantees it, but the PBP path computes elfSize from two offsets in the file and passes whatever comes out, including zero. Checked at the top, before the module object exists, so that exit needs no cleanup of its own. pspautotests 314/314 with --graphics=software, and an EBOOT.PBP still boots. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01GZq8ZtJmFY7bkX5FVkr3P9 --- Core/HLE/sceKernelMemory.cpp | 2 +- Core/HLE/sceKernelModule.cpp | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Core/HLE/sceKernelMemory.cpp b/Core/HLE/sceKernelMemory.cpp index 79948fc32a..3ab7d97093 100644 --- a/Core/HLE/sceKernelMemory.cpp +++ b/Core/HLE/sceKernelMemory.cpp @@ -1663,7 +1663,7 @@ static u32 sceKernelGetMemoryBlockAddr(u32 uid, u32 addr) { PartitionMemoryBlock *block = kernelObjects.Get(uid, error); if (block) { Memory::WriteOrException_U32(block->address, addr); - return hleLogInfo(Log::sceKernel, 0, "block address: %08x", block->address); + return hleLogDebug(Log::sceKernel, 0, "block address: %08x", block->address); } else { return hleLogError(Log::sceKernel, 0, "failed"); } diff --git a/Core/HLE/sceKernelModule.cpp b/Core/HLE/sceKernelModule.cpp index 8cdb363bf0..72711b87f7 100644 --- a/Core/HLE/sceKernelModule.cpp +++ b/Core/HLE/sceKernelModule.cpp @@ -1095,6 +1095,15 @@ enum : u32 { // filename is only used for dumping/metadata. static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 loadAddress, bool fromTop, std::string *error_string, u32 *magic, std::string_view filename, u32 &error) { + // The magic reads below need four bytes, and the ~SCE branch another four after that. Everything + // downstream checks its own sizes; this is just so we can look at the magic at all. The PBP path + // in __KernelLoadModule computes elfSize from two offsets in the file and doesn't floor it. + if (elfSize < 2 * sizeof(u32)) { + *error_string = "ELF file truncated - can't load"; + error = SCE_KERNEL_ERROR_FILEERR; + return nullptr; + } + PSPModule *module = new PSPModule(); kernelObjects.Create(module); loadedModules.insert(module->GetUID()); @@ -1173,6 +1182,9 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load // with negative size, of course. if (decryptedSize < 0 || decryptedSize > 24 * 1024 * 1024 || decryptedSize > (int)maxElfSize) { *error_string = StringFromFormat("ELF/PRX corrupt, unreasonable decrypted size: %d", (u32)decryptedSize); + delete [] newptr; + module->Cleanup(); + kernelObjects.Destroy(module->GetUID()); // TODO: Might be the wrong error code. error = SCE_KERNEL_ERROR_FILEERR; return nullptr; @@ -1194,6 +1206,9 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load // Bail out cleanly here rather than falling through to parse whatever's left // in the buffer (still compressed, not a valid ELF) as if it were real code. *error_string = StringFromFormat("Module '%s' decompression failed", head->modname); + delete [] newptr; + module->Cleanup(); + kernelObjects.Destroy(module->GetUID()); // TODO: Might be the wrong error code. error = SCE_KERNEL_ERROR_FILEERR; return nullptr; @@ -1214,6 +1229,10 @@ static PSPModule *__KernelLoadELFFromPtr(const u8 *ptr, size_t elfSize, u32 load // This should happen for all "kernel" modules. *error_string = "Missing key"; delete [] newptr; + // ptr still points into this buffer, but nothing below reads it - and the exits further + // down all free newptr, so it has to be null by the time they're reached. + newptr = nullptr; + ptr = nullptr; module->isFake = true; strncpy(module->nm.name, head->modname, ARRAY_SIZE(module->nm.name)); module->nm.entry_addr = -1;