From 49873b1287a4690977be6e4d802b45f91ff639f3 Mon Sep 17 00:00:00 2001
From: EmptyChaos <EmptyChaos@users.noreply.github.com>
Date: Mon, 23 Jan 2017 21:05:11 +0000
Subject: [PATCH] MMU/PatchEngine: Fix potential crash during stack probe

TryReadInstruction doesn't validate the address it resolves, that
can result in Memory::GetPointer failing and returning nullptr
which then leads to a nullptr dereference and a crash.

Created PowerPC::HostIsInstructionRAMAddress which works the same
way as PowerPC::HostIsRAMAddress for the IBAT.
---
 Source/Core/Core/PatchEngine.cpp   |  6 +-----
 Source/Core/Core/PowerPC/MMU.cpp   | 24 +++++++++++++++++-------
 Source/Core/Core/PowerPC/PowerPC.h |  2 ++
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/Source/Core/Core/PatchEngine.cpp b/Source/Core/Core/PatchEngine.cpp
index d2b9e74825..58207da6c5 100644
--- a/Source/Core/Core/PatchEngine.cpp
+++ b/Source/Core/Core/PatchEngine.cpp
@@ -223,11 +223,7 @@ static bool IsStackSane()
     return false;
 
   // Check the link register makes sense (that it points to a valid IBAT address)
-  auto insn = PowerPC::TryReadInstruction(PowerPC::HostRead_U32(next_SP + 4));
-  if (!insn.valid || !insn.hex)
-    return false;
-
-  return true;
+  return PowerPC::HostIsInstructionRAMAddress(PowerPC::HostRead_U32(next_SP + 4));
 }
 
 bool ApplyFramePatches()
diff --git a/Source/Core/Core/PowerPC/MMU.cpp b/Source/Core/Core/PowerPC/MMU.cpp
index 2fbb94fd85..d1ac07713c 100644
--- a/Source/Core/Core/PowerPC/MMU.cpp
+++ b/Source/Core/Core/PowerPC/MMU.cpp
@@ -622,19 +622,18 @@ bool IsOptimizableRAMAddress(const u32 address)
   return (bat_result & 2) != 0;
 }
 
-bool HostIsRAMAddress(u32 address)
+template <XCheckTLBFlag flag>
+static bool IsRAMAddress(u32 address, bool translate)
 {
-  bool performTranslation = UReg_MSR(MSR).DR;
-  int segment = address >> 28;
-  if (performTranslation)
+  if (translate)
   {
-    auto translate_address = TranslateAddress<FLAG_NO_EXCEPTION>(address);
+    auto translate_address = TranslateAddress<flag>(address);
     if (!translate_address.Success())
       return false;
     address = translate_address.address;
-    segment = address >> 28;
   }
 
+  u32 segment = address >> 28;
   if (segment == 0x0 && (address & 0x0FFFFFFF) < Memory::REALRAM_SIZE)
     return true;
   else if (Memory::m_pEXRAM && segment == 0x1 && (address & 0x0FFFFFFF) < Memory::EXRAM_SIZE)
@@ -646,6 +645,17 @@ bool HostIsRAMAddress(u32 address)
   return false;
 }
 
+bool HostIsRAMAddress(u32 address)
+{
+  return IsRAMAddress<FLAG_NO_EXCEPTION>(address, UReg_MSR(MSR).DR);
+}
+
+bool HostIsInstructionRAMAddress(u32 address)
+{
+  // Instructions are always 32bit aligned.
+  return !(address & 3) && IsRAMAddress<FLAG_OPCODE_NO_EXCEPTION>(address, UReg_MSR(MSR).IR);
+}
+
 void DMA_LCToMemory(const u32 memAddr, const u32 cacheAddr, const u32 numBlocks)
 {
   // TODO: It's not completely clear this is the right spot for this code;
@@ -1240,7 +1250,7 @@ void IBATUpdated()
 template <const XCheckTLBFlag flag>
 static TranslateAddressResult TranslateAddress(const u32 address)
 {
-  u32 bat_result = (flag == FLAG_OPCODE ? ibat_table : dbat_table)[address >> BAT_INDEX_SHIFT];
+  u32 bat_result = (IsOpcodeFlag(flag) ? ibat_table : dbat_table)[address >> BAT_INDEX_SHIFT];
   if (bat_result & 1)
   {
     u32 result_addr = (bat_result & ~3) | (address & 0x0001FFFF);
diff --git a/Source/Core/Core/PowerPC/PowerPC.h b/Source/Core/Core/PowerPC/PowerPC.h
index 7bac1d97a2..d269c1462a 100644
--- a/Source/Core/Core/PowerPC/PowerPC.h
+++ b/Source/Core/Core/PowerPC/PowerPC.h
@@ -220,6 +220,8 @@ void HostWrite_U64(const u64 var, const u32 address);
 // Returns whether a read or write to the given address will resolve to a RAM
 // access given the current CPU state.
 bool HostIsRAMAddress(const u32 address);
+// Same as HostIsRAMAddress, but uses IBAT instead of DBAT.
+bool HostIsInstructionRAMAddress(u32 address);
 
 std::string HostGetString(u32 em_address, size_t size = 0);