From fa3eff96fbc5aa34cef848a5a6b897f3624be670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 19 Oct 2026 20:54:29 +0200 Subject: [PATCH] Display: Don't read past memory presenting a framebuffer from RAM The PSP accepts display strides that run past the end of VRAM, and negative ones (setframebuf.expected has -64 and 2048), and presenting straight from memory read all 272 rows of whatever stride was set. Without a matching framebuffer, that's a read far outside PSP memory. Now the whole image has to be in memory, or the display shows black, on the hardware backends and the software renderer. A savestate's display framebuffer with an address outside RAM and VRAM, or an invalid format, turns the display off instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- Core/HLE/sceDisplay.cpp | 15 +++++++++++++++ GPU/Common/FramebufferManagerCommon.cpp | 14 ++++++++++++-- GPU/Software/SoftGpu.cpp | 4 +++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp index 95a18e154e..66abbb1c96 100644 --- a/Core/HLE/sceDisplay.cpp +++ b/Core/HLE/sceDisplay.cpp @@ -237,6 +237,17 @@ struct GPUStatistics_v0 { int moreInts[15]; }; +// A savestate's display framebuffer, straight from the file. The stride can be anything (the PSP +// takes negative ones, and GE dump playback sets others): presenting checks that. +static void SanitizeLoadedFramebuf(FrameBufferState &fb) { + const bool validAddr = fb.topaddr == 0 || Memory::IsRAMAddress(fb.topaddr) || Memory::IsVRAMAddress(fb.topaddr); + if (!validAddr || fb.fmt < GE_FORMAT_565 || fb.fmt > GE_FORMAT_8888) { + ERROR_LOG(Log::SaveState, "Savestate has an invalid display framebuffer (%08x, format %d), turning the display off", fb.topaddr, (int)fb.fmt); + fb.topaddr = 0; + fb.fmt = GE_FORMAT_8888; + } +} + void __DisplayDoState(PointerWrap &p) { auto s = p.Section("sceDisplay", 1, 8); if (!s) @@ -244,6 +255,10 @@ void __DisplayDoState(PointerWrap &p) { Do(p, framebuf); Do(p, latchedFramebuf); + if (p.mode == p.MODE_READ) { + SanitizeLoadedFramebuf(framebuf); + SanitizeLoadedFramebuf(latchedFramebuf); + } Do(p, framebufIsLatched); DisplayHWDoState(p, s <= 2); Do(p, hasSetMode); diff --git a/GPU/Common/FramebufferManagerCommon.cpp b/GPU/Common/FramebufferManagerCommon.cpp index 74d1e319ed..e26412365f 100644 --- a/GPU/Common/FramebufferManagerCommon.cpp +++ b/GPU/Common/FramebufferManagerCommon.cpp @@ -1479,6 +1479,16 @@ void FramebufferManagerCommon::CopyFramebufferForColorTexture(VirtualFramebuffer } } +// Whether the 480x272 the display shows from addr is all in memory. The PSP takes strides that +// aren't: negative ones (rows going up, which this doesn't handle), and larger than VRAM has room for. +static bool IsValidDisplayRange(u32 addr, int stride, GEBufferFormat format) { + if (stride <= 0) { + return false; + } + const u32 bpp = BufferFormatBytesPerPixel(format); + return Memory::IsValidRange(addr, ((272 - 1) * (u32)stride + 480) * bpp); +} + Draw::Texture *FramebufferManagerCommon::MakePixelTexture(const u8 *srcPixels, GEBufferFormat srcPixelFormat, int srcStride, int width, int height) { Draw::DataFormat depthFormat = Draw::DataFormat::UNDEFINED; @@ -1787,7 +1797,7 @@ void FramebufferManagerCommon::PrepareCopyDisplayToOutput(const DisplayLayoutCon VirtualFramebuffer *vfb = FindDisplayVFB(fbaddr, &offsetX, &offsetY); if (!vfb) { - if (Memory::IsValidAddress(fbaddr)) { + if (IsValidDisplayRange(fbaddr, displayStride_, displayFormat_)) { // The game is displaying something directly from RAM. In GTA, it's decoded video. // If successful, this effectively calls presentation_->NotifyPresent(); if (!DrawFramebufferToOutput(config, Memory::GetPointerUnchecked(fbaddr), displayStride_, displayFormat_)) { @@ -3018,7 +3028,7 @@ void FramebufferManagerCommon::NotifyBlockTransferAfter(u32 dstBasePtr, int dstS if (!useBufferedRendering_ && dstStride >= 480 && width >= 480 && height == 272) { bool isPrevDisplayBuffer = PrevDisplayFramebufAddr() == dstBasePtr; bool isDisplayBuffer = CurrentDisplayFramebufAddr() == dstBasePtr; - if (isPrevDisplayBuffer || isDisplayBuffer) { + if ((isPrevDisplayBuffer || isDisplayBuffer) && IsValidDisplayRange(dstBasePtr, dstStride, displayFormat_)) { FlushBeforeCopy(); // HACK DrawFramebufferToOutput(displayLayoutConfigCopy_, Memory::GetPointerUnchecked(dstBasePtr), dstStride, displayFormat_); diff --git a/GPU/Software/SoftGpu.cpp b/GPU/Software/SoftGpu.cpp index 445a54ea5e..aa26ae31be 100644 --- a/GPU/Software/SoftGpu.cpp +++ b/GPU/Software/SoftGpu.cpp @@ -647,7 +647,9 @@ void SoftGPU::CopyToCurrentFboFromDisplayRam(const DisplayLayoutConfig &config, outputFlags |= OutputFlags::PILLARBOX; } } - } else if (!Memory::IsValidAddress(displayFramebuf_) || srcwidth == 0 || srcheight == 0) { + } else if (srcwidth == 0 || srcheight == 0 || displayStride_ < 0 || !Memory::IsValidRange(displayFramebuf_, (displayStride_ == 0 ? srcwidth : displayStride_) * srcheight * (displayFormat_ == GE_FORMAT_8888 ? 4 : 2))) { + // Nothing, or not all of it in memory: the PSP takes strides that run past the end of VRAM, or + // negative ones (rows going up.) hasImage = false; u1 = 1.0f; } else if (displayFormat_ == GE_FORMAT_8888) {