Skip to content

Commit bdd2029

Browse files
Core: Fix memory range guard check.
Was allowing sizes with the high bits set, which could cause all kinds of weird issues and crashes.
1 parent 9e7625c commit bdd2029

File tree

3 files changed

+60
-16
lines changed

3 files changed

+60
-16
lines changed

Core/MemMap.h

+7-10
Original file line numberDiff line numberDiff line change
@@ -306,16 +306,13 @@ inline bool IsValidAddress(const u32 address) {
306306
inline u32 ValidSize(const u32 address, const u32 requested_size) {
307307
u32 max_size;
308308
if ((address & 0x3E000000) == 0x08000000) {
309-
max_size = 0x08000000 + g_MemorySize - address;
310-
}
311-
else if ((address & 0x3F800000) == 0x04000000) {
312-
max_size = 0x04800000 - address;
313-
}
314-
else if ((address & 0xBFFF0000) == 0x00010000) {
315-
max_size = 0x00014000 - address;
316-
}
317-
else if ((address & 0x3F000000) >= 0x08000000 && (address & 0x3F000000) < 0x08000000 + g_MemorySize) {
318-
max_size = 0x08000000 + g_MemorySize - address;
309+
max_size = 0x08000000 + g_MemorySize - (address & 0x3E000000);
310+
} else if ((address & 0x3F800000) == 0x04000000) {
311+
max_size = 0x04800000 - (address & 0x3F800000);
312+
} else if ((address & 0xBFFF0000) == 0x00010000) {
313+
max_size = 0x00014000 - (address & 0xBFFF0000);
314+
} else if ((address & 0x3F000000) >= 0x08000000 && (address & 0x3F000000) < 0x08000000 + g_MemorySize) {
315+
max_size = 0x08000000 + g_MemorySize - (address & 0x3F000000);
319316
} else {
320317
max_size = 0;
321318
}

GPU/Common/FramebufferCommon.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1409,7 +1409,7 @@ VirtualFramebuffer *FramebufferManagerCommon::CreateRAMFramebuffer(uint32_t fbAd
14091409
vfb->fbo = draw_->CreateFramebuffer({ vfb->renderWidth, vfb->renderHeight, 1, 1, true, (Draw::FBColorDepth)vfb->colorDepth });
14101410
vfbs_.push_back(vfb);
14111411

1412-
size_t byteSize = ColorBufferByteSize(vfb);
1412+
u32 byteSize = ColorBufferByteSize(vfb);
14131413
if (fbAddress + byteSize > framebufRangeEnd_) {
14141414
framebufRangeEnd_ = fbAddress + byteSize;
14151415
}

unittest/UnitTest.cpp

+52-5
Original file line numberDiff line numberDiff line change
@@ -31,21 +31,20 @@
3131
#include <string>
3232
#include <sstream>
3333

34-
#include "Common/BitScan.h"
35-
#include "Core/MIPS/MIPSVFPUUtils.h"
36-
3734
#include "base/NativeApp.h"
3835
#include "base/logging.h"
3936
#include "input/input_state.h"
4037
#include "ext/disarm.h"
4138
#include "math/math_util.h"
4239
#include "util/text/parsers.h"
4340

44-
#include "Common/CPUDetect.h"
4541
#include "Common/ArmEmitter.h"
42+
#include "Common/BitScan.h"
43+
#include "Common/CPUDetect.h"
4644
#include "Core/Config.h"
47-
#include "Core/MIPS/MIPSVFPUUtils.h"
4845
#include "Core/FileSystems/ISOFileSystem.h"
46+
#include "Core/MemMap.h"
47+
#include "Core/MIPS/MIPSVFPUUtils.h"
4948
#include "GPU/Common/TextureDecoder.h"
5049

5150
#include "unittest/JitHarness.h"
@@ -485,6 +484,53 @@ bool TestCLZ() {
485484
return true;
486485
}
487486

487+
static bool TestMemMap() {
488+
Memory::g_MemorySize = Memory::RAM_DOUBLE_SIZE;
489+
490+
enum class Flags {
491+
NO_KERNEL = 0,
492+
ALLOW_KERNEL = 1,
493+
};
494+
struct Range {
495+
uint32_t base;
496+
uint32_t size;
497+
Flags flags;
498+
};
499+
static const Range ranges[] = {
500+
{ 0x08000000, Memory::RAM_DOUBLE_SIZE, Flags::ALLOW_KERNEL },
501+
{ 0x00010000, Memory::SCRATCHPAD_SIZE, Flags::NO_KERNEL },
502+
{ 0x04000000, 0x00800000, Flags::NO_KERNEL },
503+
};
504+
static const uint32_t extraBits[] = {
505+
0x00000000,
506+
0x40000000,
507+
0x80000000,
508+
};
509+
510+
for (const auto &range : ranges) {
511+
size_t testBits = range.flags == Flags::ALLOW_KERNEL ? 3 : 2;
512+
for (size_t i = 0; i < testBits; ++i) {
513+
uint32_t base = range.base | extraBits[i];
514+
515+
EXPECT_TRUE(Memory::IsValidAddress(base));
516+
EXPECT_TRUE(Memory::IsValidAddress(base + range.size - 1));
517+
EXPECT_FALSE(Memory::IsValidAddress(base + range.size));
518+
EXPECT_FALSE(Memory::IsValidAddress(base - 1));
519+
520+
EXPECT_EQ_HEX(Memory::ValidSize(base, range.size), range.size);
521+
EXPECT_EQ_HEX(Memory::ValidSize(base, range.size + 1), range.size);
522+
EXPECT_EQ_HEX(Memory::ValidSize(base, range.size - 1), range.size - 1);
523+
EXPECT_EQ_HEX(Memory::ValidSize(base, 0), 0);
524+
EXPECT_EQ_HEX(Memory::ValidSize(base, 0x80000001), range.size);
525+
EXPECT_EQ_HEX(Memory::ValidSize(base, 0x40000001), range.size);
526+
EXPECT_EQ_HEX(Memory::ValidSize(base, 0x20000001), range.size);
527+
EXPECT_EQ_HEX(Memory::ValidSize(base, 0x10000001), range.size);
528+
}
529+
}
530+
531+
return true;
532+
}
533+
488534
typedef bool (*TestFunc)();
489535
struct TestItem {
490536
const char *name;
@@ -518,6 +564,7 @@ TestItem availableTests[] = {
518564
TEST_ITEM(ParseLBN),
519565
TEST_ITEM(QuickTexHash),
520566
TEST_ITEM(CLZ),
567+
TEST_ITEM(MemMap),
521568
};
522569

523570
int main(int argc, const char *argv[]) {

0 commit comments

Comments
 (0)