Merge pull request #22947 from hrydgard/gpu-hardening

GE, texture packs, shaders: Bound memory access from GE commands and shared content

GPU/Common/PostShader.cpp

@@ -31,6 +31,7 @@
 #include "Common/StringUtils.h"
 
 #include "Core/System.h"
+#include "Core/Util/PathUtil.h"
 #include "GPU/Common/PostShader.h"
 
 static std::vector<ShaderInfo> shaderInfo;
@@ -154,12 +155,17 @@ void LoadPostShaderInfo(Draw::DrawContext *draw, const std::vector<Path> &direct
 					section.Get("Name", &info.name);
 					section.Get("Parent", &info.parent);
 					section.Get("Visible", &info.visible);
-					temp.clear();
-					section.Get("Fragment", &temp);
-					info.fragmentShaderFile = path / temp;
-					temp.clear();
-					section.Get("Vertex", &temp);
-					info.vertexShaderFile = path / temp;
+					std::string fragment;
+					std::string vertex;
+					section.Get("Fragment", &fragment);
+					section.Get("Vertex", &vertex);
+					// Shader inis come from downloads, so keep them to their own folder.
+					if (HasParentDirComponent(fragment) || HasParentDirComponent(vertex)) {
+						WARN_LOG(Log::G3D, "Ignoring shader '%s' with a parent dir in its paths", info.section.c_str());
+						continue;
+					}
+					info.fragmentShaderFile = path / fragment;
+					info.vertexShaderFile = path / vertex;
 					section.Get("OutputResolution", &info.outputResolution);
 					section.Get("Upscaling", &info.isUpscalingFilter);
 					section.Get("SSAA", &info.SSAAFilterLevel);
@@ -221,19 +227,33 @@ void LoadPostShaderInfo(Draw::DrawContext *draw, const std::vector<Path> &direct
 					section.Get("Hidden", &info.hidden);
 					std::string cbufferFilename;
 					if (section.Get("ConstantBuffer", &cbufferFilename)) {
+						if (HasParentDirComponent(cbufferFilename)) {
+							WARN_LOG(Log::G3D, "Ignoring texture shader '%s' with a parent dir in its paths", info.section.c_str());
+							continue;
+						}
 						Path cbufferPath = path / cbufferFilename;
 						info.constantBuffer = cbufferPath;
 					}
 					if (section.Get("Compute", &temp)) {
+						if (HasParentDirComponent(temp)) {
+							WARN_LOG(Log::G3D, "Ignoring texture shader '%s' with a parent dir in its paths", info.section.c_str());
+							continue;
+						}
 						info.computeShaderFile = path / temp;
 						info.computeShaderFiles.push_back(info.computeShaderFile);
+						bool badPath = false;
 						for (int computeIndex = 2; computeIndex <= 4; ++computeIndex) {
 							temp.clear();
 							if (section.Get(StringFromFormat("Compute%d", computeIndex).c_str(), &temp)) {
+								if (HasParentDirComponent(temp)) {
+									badPath = true;
+								}
 								info.computeShaderFiles.push_back(path / temp);
 							}
 						}
-						if (info.scaleFactor >= 2 && info.scaleFactor < 8) {
+						if (badPath) {
+							WARN_LOG(Log::G3D, "Ignoring texture shader '%s' with a parent dir in its paths", info.section.c_str());
+						} else if (info.scaleFactor >= 2 && info.scaleFactor < 8) {
 							appendTextureShader(info);
 						}
 					} else {

GPU/Common/ReplacedTexture.cpp

@@ -1015,9 +1015,21 @@ bool ReplacedTexture::CopyLevelTo(int level, uint8_t *out, size_t outDataSize, i
 		WARN_LOG(Log::TexReplacement, "Level %d is empty", level);
 		return false;
 	}
+	if (info.w > outW || info.h > outH) {
+		WARN_LOG(Log::TexReplacement, "Level %d (%dx%d) is bigger than its padded size %dx%d", level, info.w, info.h, outW, outH);
+		return false;
+	}
 
 	int blockSize;
-	if (!Draw::DataFormatIsBlockCompressed(fmt, &blockSize)) {
+	const bool blockCompressed = Draw::DataFormatIsBlockCompressed(fmt, &blockSize);
+	const size_t needed = blockCompressed ? (size_t)((outW + 3) / 4) * ((outH + 3) / 4) * blockSize : (size_t)rowPitch * (outH - 1) + outW * 4;
+	// A zero size disables the check, like for the other copies.
+	if (outDataSize != 0 && outDataSize < needed) {
+		WARN_LOG(Log::TexReplacement, "Level %d doesn't fit the buffer (%d < %d)", level, (int)outDataSize, (int)needed);
+		return false;
+	}
+
+	if (!blockCompressed) {
 		if (fmt != Draw::DataFormat::R8G8B8A8_UNORM) {
 			ERROR_LOG(Log::TexReplacement, "Unexpected linear data format");
 			return false;

GPU/Common/TextureReplacer.cpp

@@ -413,8 +413,9 @@ bool TextureReplacer::LoadIniValues(IniFile &ini, VFSBackend *dir, bool isOverri
 			std::string_view v = line.Value();
 			if (sscanf(k, "%16llx%8x_%d", &key.cachekey, &key.hash, &level) >= 1) {
 				// Reject path traversal: a "../" component could make us read
-				// or write files outside the texture pack directory.
-				if (HasParentDirComponent(v)) {
+				// or write files outside the texture pack directory. '|' separates mip levels in the alias,
+				// so it would let a later level skip this check.
+				if (HasParentDirComponent(v) || v.find('|') != std::string_view::npos) {
 					ERROR_LOG(Log::TexReplacement, "Ignoring texture filename with parent dir component: %s", std::string(v).c_str());
 					continue;
 				}
@@ -521,6 +522,12 @@ void TextureReplacer::ParseHashRange(const std::string &key, const std::string &
 		return;
 	}
 
+	// Generous, PSP textures are at most 512x512. Values of 65536 or more would also spill into the other fields of the key.
+	if (fromW > 1024 || fromH > 1024) {
+		ERROR_LOG(Log::TexReplacement, "Ignoring invalid hashrange %s = %s, source bigger than 1024x1024", key.c_str(), value.c_str());
+		return;
+	}
+
 	if (toW > fromW || toH > fromH) {
 		ERROR_LOG(Log::TexReplacement, "Ignoring invalid hashrange %s = %s, range bigger than source", key.c_str(), value.c_str());
 		return;
@@ -934,13 +941,13 @@ void TextureReplacer::NotifyTextureDecoded(ReplacedTexture *texture, const Repla
 	// Only save the hashed portion of the PNG, in case we have specified it in the ini file.
 	int lookupW;
 	int lookupH;
-	if (LookupHashRange(replacedInfo.addr, origW, origH, &lookupW, &lookupH)) {
+	if (LookupHashRange(replacedInfo.addr, origW, origH, &lookupW, &lookupH) && lookupW > 0 && lookupH > 0 && lookupW <= origW && lookupH <= origH) {
 		w = lookupW * (scaledW / origW);
 		h = lookupH * (scaledH / origH);
 	}
 
 	constexpr int bpp = 4;
-	size_t saveBufSize = w * h * bpp;
+	size_t saveBufSize = (size_t)w * h * bpp;
 	u8 *saveBuf = (u8 *)malloc(saveBufSize);
 	if (!saveBuf) {
 		ERROR_LOG(Log::TexReplacement, "Failed to allocated %d bytes of memory for saving a texture", (int)saveBufSize);
@@ -950,7 +957,7 @@ void TextureReplacer::NotifyTextureDecoded(ReplacedTexture *texture, const Repla
 	// Copy data to a buffer so we can send it to the thread. Might as well compact-away the pitch
 	// while we're at it.
 	for (int y = 0; y < h; y++) {
-		memcpy(saveBuf + y * w * bpp, (const u8 *)data + y * srcPitch, w * bpp);
+		memcpy(saveBuf + (size_t)y * w * bpp, (const u8 *)data + (size_t)y * srcPitch, w * bpp);
 	}
 
 	SaveTextureTask *task = new SaveTextureTask(std::move(saveBuf));

GPU/Common/VertexDecoderCommon.cpp

@@ -201,6 +201,25 @@ void GetIndexBounds(const void *inds, int count, u32 vertType, u16 *indexLowerBo
 	}
 }
 
+bool IsValidIndexedDrawRange(u32 vertexAddr, u32 indexAddr, int count, u32 vertType, int vertexSize) {
+	const u32 idx = vertType & GE_VTYPE_IDX_MASK;
+	const int indexShift = (idx >> GE_VTYPE_IDX_SHIFT) - 1;
+	if (!Memory::IsValidRange(indexAddr, count << indexShift)) {
+		return false;
+	}
+	// Indices are at most 16 bits, so this usually saves looking at them.
+	const u32 maxIndex = idx == GE_VTYPE_IDX_8BIT ? 0xFF : 0xFFFF;
+	if (Memory::IsValidRange(vertexAddr, (maxIndex + 1) * vertexSize)) {
+		return true;
+	}
+	if (count <= 0) {
+		return true;
+	}
+	u16 lowerBound, upperBound;
+	GetIndexBounds(Memory::GetPointerUnchecked(indexAddr), count, vertType, &lowerBound, &upperBound);
+	return Memory::IsValidRange(vertexAddr, (upperBound + 1) * vertexSize);
+}
+
 void PrintDecodedVertex(const VertexReader &vtx) {
 	if (vtx.hasNormal()) {
 		float nrm[3];

GPU/Common/VertexDecoderCommon.h

@@ -27,6 +27,7 @@
 #include "Common/Data/Collections/Hashmaps.h"
 #include "Common/Log.h"
 #include "Common/LogReporting.h"
+#include "Core/MemMap.h"
 #include "GPU/ge_constants.h"
 #include "GPU/Common/ShaderCommon.h"
 #include "GPU/GPUState.h"
@@ -92,6 +93,17 @@ struct DecVtxFormat {
 
 void GetIndexBounds(const void *inds, int count, u32 vertType, u16 *indexLowerBound, u16 *indexUpperBound);
 
+bool IsValidIndexedDrawRange(u32 vertexAddr, u32 indexAddr, int count, u32 vertType, int vertexSize);
+
+// Checks that the indices of a draw, and every vertex they can reach, are in valid memory.
+// Inline since it runs on every draw, and nearly all of them are non-indexed.
+inline bool IsValidDrawRange(u32 vertexAddr, u32 indexAddr, int count, u32 vertType, int vertexSize) {
+	if ((vertType & GE_VTYPE_IDX_MASK) == GE_VTYPE_IDX_NONE) {
+		return Memory::IsValidRange(vertexAddr, count * vertexSize);
+	}
+	return IsValidIndexedDrawRange(vertexAddr, indexAddr, count, vertType, vertexSize);
+}
+
 inline constexpr int RoundUp4(int x) {
 	return (x + 3) & ~3;
 }

GPU/GPUCommon.cpp

@@ -696,7 +696,8 @@ u32 GPUCommon::UpdateStall(int listid, u32 newstall, bool *runList) {
 	if (dl.state == PSP_GE_DL_STATE_COMPLETED)
 		return SCE_KERNEL_ERROR_ALREADY;
 
-	dl.stall = newstall & 0x0FFFFFFF;
+	// The GE fetches whole words, and an unaligned stall would never equal the PC.
+	dl.stall = newstall & 0x0FFFFFFC;
 
 	gpuStats.perFrame.numUpdateStall++;
 
@@ -875,6 +876,12 @@ bool GPUCommon::SlowRunLoop(DisplayList &list) {
 	return true;
 }
 
+// How many commands can run from pc: up to the stall, and never past the end of memory.
+static int ListDowncount(u32 pc, u32 stall) {
+	const u32 toStall = stall == 0 ? 0x0FFFFFFF : (stall - pc) / 4;
+	return (int)std::min(toStall, std::min(Memory::MaxSizeAtAddress(pc) / 4, (u32)0x0FFFFFFF));
+}
+
 // The newPC parameter is used for jumps, we don't count cycles between.
 void GPUCommon::UpdatePC(u32 currentPC, u32 newPC) {
 	// Rough estimate, 2 CPU ticks (it's double the clock rate) per GPU instruction.
@@ -884,7 +891,7 @@ void GPUCommon::UpdatePC(u32 currentPC, u32 newPC) {
 
 	// Exit the runloop and recalculate things.  This happens a lot in some games.
 	if (currentList)
-		downcount = currentList->stall == 0 ? 0x0FFFFFFF : (currentList->stall - newPC) / 4;
+		downcount = ListDowncount(newPC, currentList->stall);
 	else
 		downcount = 0;
 }
@@ -1003,7 +1010,7 @@ DLResult GPUCommon::ProcessDLQueue() {
 
 			cycleLastPC = list.pc;
 			cyclesExecuted += 60;
-			downcount = list.stall == 0 ? 0x0FFFFFFF : (list.stall - list.pc) / 4;
+			downcount = ListDowncount(list.pc, list.stall);
 			list.state = PSP_GE_DL_STATE_RUNNING;
 			list.interrupted = false;
 
@@ -1016,7 +1023,7 @@ DLResult GPUCommon::ProcessDLQueue() {
 			resumingFromDebugBreak_ = false;
 			// The bottom part of the gpuState loop below, that wasn't executed
 			// when we bailed.
-			downcount = list.stall == 0 ? 0x0FFFFFFF : (list.stall - list.pc) / 4;
+			downcount = ListDowncount(list.pc, list.stall);
 			if (gpuState == GPUSTATE_STALL && list.pc != list.stall) {
 				// Unstalled (Can this happen?)
 				gpuState = GPUSTATE_RUNNING;
@@ -1030,6 +1037,11 @@ DLResult GPUCommon::ProcessDLQueue() {
 			if (list.pc == list.stall) {
 				gpuState = GPUSTATE_STALL;
 				downcount = 0;
+			} else if (downcount == 0 && !Memory::IsValidAddress(list.pc)) {
+				// Ran (or jumped) off the end of memory. There's nothing more to run.
+				ERROR_LOG(Log::G3D, "Display list %d ran off the end of memory at %08x", listIndex, list.pc);
+				gpuState = GPUSTATE_ERROR;
+				break;
 			}
 
 			if (useFastRunLoop) {
@@ -1048,7 +1060,7 @@ DLResult GPUCommon::ProcessDLQueue() {
 				}
 			}
 
-			downcount = list.stall == 0 ? 0x0FFFFFFF : (list.stall - list.pc) / 4;
+			downcount = ListDowncount(list.pc, list.stall);
 			if (gpuState == GPUSTATE_STALL && list.pc != list.stall) {
 				// Unstalled (Can this happen?)
 				gpuState = GPUSTATE_RUNNING;
@@ -1250,7 +1262,7 @@ void GPUCommon::Execute_End(u32 op, u32 diff) {
 		Flush();
 	}
 
-	const u32 prev = Memory::ReadUnchecked_U32(currentList->pc - 4);
+	const u32 prev = Memory::IsValidAddress(currentList->pc - 4) ? Memory::ReadUnchecked_U32(currentList->pc - 4) : 0;
 	UpdatePC(currentList->pc, currentList->pc);
 	// Count in a few extra cycles on END.
 	cyclesExecuted += 60;
@@ -1480,8 +1492,7 @@ void GPUCommon::Execute_BoundingBox(u32 op, u32 diff) {
 
 	const void *inds = nullptr;
 	if (useInds) {
-		const int indexSizeShift = ((vertType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1;
-		if (!Memory::IsValidRange(gstate_c.indexAddr, count << indexSizeShift)) {
+		if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, count, vertType, dec->VertexSize())) {
 			ERROR_LOG_REPORT_ONCE(boundingboxInds, Log::G3D, "Invalid inds in bounding box check");
 			currentList->bboxResult = true;
 			return;
@@ -1680,7 +1691,7 @@ void GPUCommon::Execute_WorldMtxNum(u32 op, u32 diff) {
 	// We must record the individual data commands while debugRecording_.
 	bool fastLoad = !debugRecording_ && end > 0;
 	// Stalling in the middle of a matrix would be stupid, I doubt this check is necessary.
-	if (currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) {
+	if ((currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) || !Memory::IsValidRange(currentList->pc + 4, end * 4)) {
 		fastLoad = false;
 	}
 
@@ -1733,7 +1744,7 @@ void GPUCommon::Execute_ViewMtxNum(u32 op, u32 diff) {
 	int i = 0;
 
 	bool fastLoad = !debugRecording_ && end > 0;
-	if (currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) {
+	if ((currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) || !Memory::IsValidRange(currentList->pc + 4, end * 4)) {
 		fastLoad = false;
 	}
 
@@ -1786,7 +1797,7 @@ void GPUCommon::Execute_ProjMtxNum(u32 op, u32 diff) {
 	int i = 0;
 
 	bool fastLoad = !debugRecording_;
-	if (currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) {
+	if ((currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) || !Memory::IsValidRange(currentList->pc + 4, end * 4)) {
 		fastLoad = false;
 	}
 
@@ -1840,7 +1851,7 @@ void GPUCommon::Execute_TgenMtxNum(u32 op, u32 diff) {
 	int i = 0;
 
 	bool fastLoad = !debugRecording_ && end > 0;
-	if (currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) {
+	if ((currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) || !Memory::IsValidRange(currentList->pc + 4, end * 4)) {
 		fastLoad = false;
 	}
 
@@ -1894,7 +1905,7 @@ void GPUCommon::Execute_BoneMtxNum(u32 op, u32 diff) {
 	int i = 0;
 
 	bool fastLoad = !debugRecording_ && end > 0;
-	if (currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) {
+	if ((currentList->pc < currentList->stall && currentList->pc + end * 4 >= currentList->stall) || !Memory::IsValidRange(currentList->pc + 4, end * 4)) {
 		fastLoad = false;
 	}
 
@@ -2387,8 +2398,8 @@ void GPUCommon::DoBlockTransfer(u32 skipDrawReason) {
 	// NOTE: The sizes are only used for validity checks and memory info tracking.
 	const uint32_t src = srcBasePtr + (srcY * srcStride + srcX) * bpp;
 	const uint32_t dst = dstBasePtr + (dstY * dstStride + dstX) * bpp;
-	const uint32_t srcSize = ((height - 1) * srcStride) + width * bpp;
-	const uint32_t dstSize = ((height - 1) * dstStride) + width * bpp;
+	const uint32_t srcSize = ((height - 1) * srcStride + width) * bpp;
+	const uint32_t dstSize = ((height - 1) * dstStride + width) * bpp;
 
 	bool srcDstOverlap = src + srcSize > dst && dst + dstSize > src;
 	bool srcValid = Memory::IsValidRange(src, srcSize);

GPU/GPUCommonHW.cpp

@@ -986,7 +986,7 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) {
 	VertexDecoder *decoder = drawEngineCommon_->GetVertexDecoder(vertTypeID);
 
 	// Nothing is mapped past the end of RAM or VRAM, so a draw running off the end would fault on the first read.
-	if (!inds && !Memory::IsValidRange(gstate_c.vertexAddr, count * decoder->VertexSize())) {
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, count, vertexType, decoder->VertexSize())) {
 		ERROR_LOG(Log::G3D, "Bad vertex range %08x (%d verts of %d bytes)!", gstate_c.vertexAddr, count, decoder->VertexSize());
 		return;
 	}
@@ -1088,6 +1088,11 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) {
 	const u32_le *start = (const u32_le *)Memory::GetPointerUnchecked(currentList->pc + 4);
 	const u32_le *src = start;
 	const u32_le *stall = currentList->stall ? (const u32_le *)Memory::GetPointerUnchecked(currentList->stall) : 0;
+	// Without a stall ahead, stop at the end of memory.
+	const u32_le *end = start + Memory::MaxSizeAtAddress(currentList->pc + 4) / 4;
+	if (!stall || stall < start || stall > end) {
+		stall = end;
+	}
 
 	// Optimized submission of sequences of PRIM. Allows us to avoid going through all the mess
 	// above for each one. This can be expanded to support additional games that intersperse
@@ -1101,7 +1106,7 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) {
 	if (!useFastRunLoop_ || readsFramebufferPerDraw)
 		goto bail;  // we're either recording or stepping, or each draw has to go alone.
 
-	while (src != stall) {
+	while (src < stall) {
 		uint32_t data = *src;
 		switch (data >> 24) {
 		case GE_CMD_PRIM:
@@ -1131,19 +1136,13 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) {
 
 			verts = Memory::GetPointerUnchecked(gstate_c.vertexAddr);
 			inds = nullptr;
+			if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, count, vertexType, decoder->VertexSize())) {
+				// Bad index or vertex range, leave it to Execute_Prim.
+				goto bail;
+			}
 			if ((vertexType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-				const u32 indexAddr = gstate_c.indexAddr;
-				const int indexShift = ((vertexType & GE_VTYPE_IDX_MASK) >> GE_VTYPE_IDX_SHIFT) - 1;
-				if (!Memory::IsValidRange(gstate_c.indexAddr, count << indexShift)) {
-					// Bad index range. Let's give up the fast loop.
-					goto bail;
-				}
-				inds = Memory::GetPointerUnchecked(indexAddr);
+				inds = Memory::GetPointerUnchecked(gstate_c.indexAddr);
 			} else {
-				if (!Memory::IsValidRange(gstate_c.vertexAddr, count * decoder->VertexSize())) {
-					// Bad vertex range, leave it to Execute_Prim.
-					goto bail;
-				}
 				// We can extend again after submitting a normal draw.
 				canExtend = isTriangle;
 			}
@@ -1267,7 +1266,7 @@ void GPUCommonHW::Execute_Prim(u32 op, u32 diff) {
 		{
 			// A bone matrix probably. If not we bail.
 			const u32 target = gstate_c.getRelativeAddress(data & 0x00FFFFFC);
-			if (currentList->HardwareCallDepth() < 2 &&
+			if (currentList->HardwareCallDepth() < 2 && Memory::IsValidRange(target, 13 * 4) &&
 				(Memory::ReadUnchecked_U32(target) >> 24) == GE_CMD_BONEMATRIXDATA &&
 				(Memory::ReadUnchecked_U32(target + 11 * 4) >> 24) == GE_CMD_BONEMATRIXDATA &&
 				(Memory::ReadUnchecked_U32(target + 12 * 4) >> 24) == GE_CMD_RET &&
@@ -1352,17 +1351,14 @@ void GPUCommonHW::Execute_Bezier(u32 op, u32 diff) {
 	const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr);
 	const void *indices = NULL;
 	const u32 vertType = gstate.vertType;
-	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-		if (!Memory::IsValidAddress(gstate_c.indexAddr)) {
-			ERROR_LOG(Log::G3D, "Bad index address %08x!", gstate_c.indexAddr);
-			return;
-		}
-		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
-	} else if (!Memory::IsValidRange(gstate_c.vertexAddr, (op & 0xFF) * ((op >> 8) & 0xFF) * drawEngineCommon_->GetVertexDecoder(GetVertTypeID(vertType, gstate.getUVGenMode()))->VertexSize())) {
-		// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
-		ERROR_LOG(Log::G3D, "Bad control point range %08x!", gstate_c.vertexAddr);
+	// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, (op & 0xFF) * ((op >> 8) & 0xFF), vertType, drawEngineCommon_->GetVertexDecoder(GetVertTypeID(vertType, gstate.getUVGenMode()))->VertexSize())) {
+		ERROR_LOG(Log::G3D, "Bad control point range %08x (indices %08x)!", gstate_c.vertexAddr, gstate_c.indexAddr);
 		return;
 	}
+	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
+		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
+	}
 
 	if (vertTypeIsSkinningEnabled(vertType)) {
 		DEBUG_LOG_REPORT(Log::G3D, "Unusual bezier/spline vtype: %08x, morph: %d, bones: %d", vertType, (vertType & GE_VTYPE_MORPHCOUNT_MASK) >> GE_VTYPE_MORPHCOUNT_SHIFT, vertTypeGetNumBoneWeights(vertType));
@@ -1429,17 +1425,14 @@ void GPUCommonHW::Execute_Spline(u32 op, u32 diff) {
 	const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr);
 	const void *indices = NULL;
 	const u32 vertType = gstate.vertType;
-	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-		if (!Memory::IsValidAddress(gstate_c.indexAddr)) {
-			ERROR_LOG(Log::G3D, "Bad index address %08x!", gstate_c.indexAddr);
-			return;
-		}
-		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
-	} else if (!Memory::IsValidRange(gstate_c.vertexAddr, (op & 0xFF) * ((op >> 8) & 0xFF) * drawEngineCommon_->GetVertexDecoder(GetVertTypeID(vertType, gstate.getUVGenMode()))->VertexSize())) {
-		// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
-		ERROR_LOG(Log::G3D, "Bad control point range %08x!", gstate_c.vertexAddr);
+	// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, (op & 0xFF) * ((op >> 8) & 0xFF), vertType, drawEngineCommon_->GetVertexDecoder(GetVertTypeID(vertType, gstate.getUVGenMode()))->VertexSize())) {
+		ERROR_LOG(Log::G3D, "Bad control point range %08x (indices %08x)!", gstate_c.vertexAddr, gstate_c.indexAddr);
 		return;
 	}
+	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
+		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
+	}
 
 	if (vertTypeIsSkinningEnabled(vertType)) {
 		WARN_LOG_ONCE(unusualcurve, Log::G3D, "Unusual bezier/spline vtype: %08x, morph: %d, bones: %d", vertType, (vertType & GE_VTYPE_MORPHCOUNT_MASK) >> GE_VTYPE_MORPHCOUNT_SHIFT, vertTypeGetNumBoneWeights(vertType));

GPU/Software/BinManager.cpp

@@ -262,6 +262,17 @@ void BinManager::UpdateState() {
 		drawTargetAddr_ = gstate.getFrameBufAddress();
 		DrawingCoords scissorTL(gstate.getScissorX1(), gstate.getScissorY1());
 		DrawingCoords scissorBR(std::min(gstate.getScissorX2(), gstate.getRegionX2()), std::min(gstate.getScissorY2(), gstate.getRegionY2()));
+		// fb.data points into VRAM's mirrors, which end 8MB from its start. A wide 32-bit stride can
+		// reach past that within 1024 rows, so leave out the rows that would.
+		const int fbStride = gstate.FrameBufStride();
+		if (fbStride > 0) {
+			const int bpp = gstate.FrameBufFormat() == GE_FORMAT_8888 ? 4 : 2;
+			const int bytesLeft = 0x00800000 - (int)gstate.getFrameBufRawAddress() - (scissorBR.x + 1) * bpp;
+			const int maxY = bytesLeft / (fbStride * bpp);
+			if (scissorBR.y > maxY) {
+				scissorBR.y = maxY;
+			}
+		}
 		ScreenCoords screenScissorTL = TransformUnit::DrawingToScreen(scissorTL, 0);
 		ScreenCoords screenScissorBR = TransformUnit::DrawingToScreen(scissorBR, 0);
 

GPU/Software/SoftGpu.cpp

@@ -969,17 +969,14 @@ void SoftGPU::Execute_Prim(u32 op, u32 diff) {
 	const void *indices = NULL;
 
 	const u32 vertType = gstate.vertType;
-	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-		if (!Memory::IsValidAddress(gstate_c.indexAddr)) {
-			ERROR_LOG(Log::G3D, "Software: Bad index address %08x!", gstate_c.indexAddr);
-			return;
-		}
-		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
-	} else if (!Memory::IsValidRange(gstate_c.vertexAddr, count * drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
-		// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
-		ERROR_LOG(Log::G3D, "Software: Bad vertex range %08x (%d verts)!", gstate_c.vertexAddr, count);
+	// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, count, vertType, drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
+		ERROR_LOG(Log::G3D, "Software: Bad vertex range %08x (%d verts, indices %08x)!", gstate_c.vertexAddr, count, gstate_c.indexAddr);
 		return;
 	}
+	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
+		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
+	}
 
 	if (gstate.isModeThrough()) {
 		cyclesExecuted += EstimateFillCycles(prim, verts, indices, count, drawEngine_->FindVertexDecoder(vertType), vertType);
@@ -1052,17 +1049,14 @@ void SoftGPU::Execute_Bezier(u32 op, u32 diff) {
 	const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr);
 	const void *indices = NULL;
 	const u32 vertType = gstate.vertType;
-	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-		if (!Memory::IsValidAddress(gstate_c.indexAddr)) {
-			ERROR_LOG(Log::G3D, "Bad index address %08x!", gstate_c.indexAddr);
-			return;
-		}
-		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
-	} else if (!Memory::IsValidRange(gstate_c.vertexAddr, (op & 0xFF) * ((op >> 8) & 0xFF) * drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
-		// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
-		ERROR_LOG(Log::G3D, "Bad control point range %08x!", gstate_c.vertexAddr);
+	// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, (op & 0xFF) * ((op >> 8) & 0xFF), vertType, drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
+		ERROR_LOG(Log::G3D, "Bad control point range %08x (indices %08x)!", gstate_c.vertexAddr, gstate_c.indexAddr);
 		return;
 	}
+	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
+		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
+	}
 
 	if ((vertType & GE_VTYPE_MORPHCOUNT_MASK) || vertTypeIsSkinningEnabled(vertType)) {
 		DEBUG_LOG(Log::G3D, "Unusual bezier/spline vtype: %08x, morph: %d, bones: %d", vertType, (vertType & GE_VTYPE_MORPHCOUNT_MASK) >> GE_VTYPE_MORPHCOUNT_SHIFT, vertTypeGetNumBoneWeights(vertType));
@@ -1112,17 +1106,14 @@ void SoftGPU::Execute_Spline(u32 op, u32 diff) {
 	const void *control_points = Memory::GetPointerUnchecked(gstate_c.vertexAddr);
 	const void *indices = NULL;
 	const u32 vertType = gstate.vertType;
-	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
-		if (!Memory::IsValidAddress(gstate_c.indexAddr)) {
-			ERROR_LOG(Log::G3D, "Bad index address %08x!", gstate_c.indexAddr);
-			return;
-		}
-		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
-	} else if (!Memory::IsValidRange(gstate_c.vertexAddr, (op & 0xFF) * ((op >> 8) & 0xFF) * drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
-		// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
-		ERROR_LOG(Log::G3D, "Bad control point range %08x!", gstate_c.vertexAddr);
+	// Nothing is mapped past the end of RAM or VRAM, so this would fault on the first read.
+	if (!IsValidDrawRange(gstate_c.vertexAddr, gstate_c.indexAddr, (op & 0xFF) * ((op >> 8) & 0xFF), vertType, drawEngine_->FindVertexDecoder(vertType)->VertexSize())) {
+		ERROR_LOG(Log::G3D, "Bad control point range %08x (indices %08x)!", gstate_c.vertexAddr, gstate_c.indexAddr);
 		return;
 	}
+	if ((vertType & GE_VTYPE_IDX_MASK) != GE_VTYPE_IDX_NONE) {
+		indices = Memory::GetPointerUnchecked(gstate_c.indexAddr);
+	}
 
 	if ((vertType & GE_VTYPE_MORPHCOUNT_MASK) || vertTypeIsSkinningEnabled(vertType)) {
 		DEBUG_LOG(Log::G3D, "Unusual bezier/spline vtype: %08x, morph: %d, bones: %d", vertType, (vertType & GE_VTYPE_MORPHCOUNT_MASK) >> GE_VTYPE_MORPHCOUNT_SHIFT, vertTypeGetNumBoneWeights(vertType));

GPU/Vulkan/TextureCacheVulkan.cpp

@@ -366,6 +366,13 @@ bool TextureCacheVulkan::LoadConstantBuffer(VulkanContext *vulkan, VkCommandBuff
 		ClearScalingShaders(vulkan);
 		return false;
 	}
+	// The file comes with a downloaded shader, and has to make a valid uniform buffer.
+	if (constantsSize < 16 || (constantsSize & 15) != 0 || constantsSize > vulkan->GetPhysicalDeviceProperties().properties.limits.maxUniformBufferRange) {
+		ERROR_LOG(Log::G3D, "Constant buffer file '%s' has a bad size (%d) - disabling hardware scaling", cbufferPath_.c_str(), (int)constantsSize);
+		delete[] contents;
+		ClearScalingShaders(vulkan);
+		return false;
+	}
 	textureScaleCBuffer_.Create(vulkan, "TextureScale CBuffer", constantsSize, VK_BUFFER_USAGE_UNIFORM_BUFFER_BIT | VK_BUFFER_USAGE_TRANSFER_DST_BIT);
 	VulkanPushPool *pushPool = drawEngine_->GetPushBufferForTextureData();
 	VkBuffer srcBuf = VK_NULL_HANDLE;