From 0126330530a157caab4f3483ef061515f4685cf1 Mon Sep 17 00:00:00 2001 From: Vicki Pfau Date: Wed, 8 May 2019 16:38:30 -0700 Subject: [PATCH] GBA Memory: Prevent writing to mirrored BG VRAM (fixes #743) --- CHANGES | 1 + src/gba/memory.c | 82 ++++++++++++++++++++++++++---------------------- 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/CHANGES b/CHANGES index a79bda912..371781222 100644 --- a/CHANGES +++ b/CHANGES @@ -22,6 +22,7 @@ Emulation fixes: - GB Timer: Fix timing adjustments when writing to TAC (fixes mgba.io/i/1340) - GBA Memory: Fix writing to OBJ memory in modes 3 and 5 - GBA: Fix RTC on non-standard sized ROMs (fixes mgba.io/i/1400) + - GBA Memory: Prevent writing to mirrored BG VRAM (fixes mgba.io/i/743) Other fixes: - Qt: More app metadata fixes - Qt: Fix load recent from archive (fixes mgba.io/i/1325) diff --git a/src/gba/memory.c b/src/gba/memory.c index 46aecae5b..3f037567c 100644 --- a/src/gba/memory.c +++ b/src/gba/memory.c @@ -390,11 +390,15 @@ static void GBASetActiveRegion(struct ARMCore* cpu, uint32_t address) { wait += waitstatesRegion[REGION_PALETTE_RAM]; #define LOAD_VRAM \ - if ((address & 0x0001FFFF) < SIZE_VRAM) { \ - LOAD_32(value, address & 0x0001FFFC, gba->video.vram); \ - } else { \ - LOAD_32(value, address & 0x00017FFC, gba->video.vram); \ + if ((address & 0x0001FFFF) >= SIZE_VRAM) { \ + if ((address & (SIZE_VRAM | 0x00014000)) == SIZE_VRAM && (GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3)) { \ + mLOG(GBA_MEM, GAME_ERROR, "Bad VRAM Load32: 0x%08X", address); \ + value = 0; \ + break; \ + } \ + address &= 0x00017FFC; \ } \ + LOAD_32(value, address & 0x0001FFFC, gba->video.vram); \ wait += waitstatesRegion[REGION_VRAM]; #define LOAD_OAM LOAD_32(value, address & (SIZE_OAM - 4), gba->video.oam.raw); @@ -520,11 +524,15 @@ uint32_t GBALoad16(struct ARMCore* cpu, uint32_t address, int* cycleCounter) { LOAD_16(value, address & (SIZE_PALETTE_RAM - 2), gba->video.palette); break; case REGION_VRAM: - if ((address & 0x0001FFFF) < SIZE_VRAM) { - LOAD_16(value, address & 0x0001FFFE, gba->video.vram); - } else { - LOAD_16(value, address & 0x00017FFE, gba->video.vram); + if ((address & 0x0001FFFF) >= SIZE_VRAM) { + if ((address & (SIZE_VRAM | 0x00014000)) == SIZE_VRAM && (GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3)) { + mLOG(GBA_MEM, GAME_ERROR, "Bad VRAM Load16: 0x%08X", address); + value = 0; + break; + } + address &= 0x00017FFE; } + LOAD_16(value, address & 0x0001FFFE, gba->video.vram); break; case REGION_OAM: LOAD_16(value, address & (SIZE_OAM - 2), gba->video.oam.raw); @@ -631,11 +639,15 @@ uint32_t GBALoad8(struct ARMCore* cpu, uint32_t address, int* cycleCounter) { value = ((uint8_t*) gba->video.palette)[address & (SIZE_PALETTE_RAM - 1)]; break; case REGION_VRAM: - if ((address & 0x0001FFFF) < SIZE_VRAM) { - value = ((uint8_t*) gba->video.vram)[address & 0x0001FFFF]; - } else { - value = ((uint8_t*) gba->video.vram)[address & 0x00017FFF]; + if ((address & 0x0001FFFF) >= SIZE_VRAM) { + if ((address & (SIZE_VRAM | 0x00014000)) == SIZE_VRAM && (GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3)) { + mLOG(GBA_MEM, GAME_ERROR, "Bad VRAM Load8: 0x%08X", address); + value = 0; + break; + } + address &= 0x00017FFF; } + value = ((uint8_t*) gba->video.vram)[address & 0x0001FFFF]; break; case REGION_OAM: value = ((uint8_t*) gba->video.oam.raw)[address & (SIZE_OAM - 1)]; @@ -717,20 +729,18 @@ uint32_t GBALoad8(struct ARMCore* cpu, uint32_t address, int* cycleCounter) { wait += waitstatesRegion[REGION_PALETTE_RAM]; #define STORE_VRAM \ - if ((address & 0x0001FFFF) < SIZE_VRAM) { \ - LOAD_32(oldValue, address & 0x0001FFFC, gba->video.vram); \ - if (oldValue != value) { \ - STORE_32(value, address & 0x0001FFFC, gba->video.vram); \ - gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x0001FFFC) + 2); \ - gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x0001FFFC)); \ - } \ - } else { \ - LOAD_32(oldValue, address & 0x00017FFC, gba->video.vram); \ - if (oldValue != value) { \ - STORE_32(value, address & 0x00017FFC, gba->video.vram); \ - gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x00017FFC) + 2); \ - gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x00017FFC)); \ + if ((address & 0x0001FFFF) >= SIZE_VRAM) { \ + if ((address & (SIZE_VRAM | 0x00014000)) == SIZE_VRAM && (GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3)) { \ + mLOG(GBA_MEM, GAME_ERROR, "Bad VRAM Store32: 0x%08X", address); \ + break; \ } \ + address &= 0x00017FFC; \ + } \ + LOAD_32(oldValue, address & 0x0001FFFC, gba->video.vram); \ + if (oldValue != value) { \ + STORE_32(value, address & 0x0001FFFC, gba->video.vram); \ + gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x0001FFFC) + 2); \ + gba->video.renderer->writeVRAM(gba->video.renderer, (address & 0x0001FFFC)); \ } \ wait += waitstatesRegion[REGION_VRAM]; @@ -840,18 +850,17 @@ void GBAStore16(struct ARMCore* cpu, uint32_t address, int16_t value, int* cycle } break; case REGION_VRAM: - if ((address & 0x0001FFFF) < SIZE_VRAM) { - LOAD_16(oldValue, address & 0x0001FFFE, gba->video.vram); - if (value != oldValue) { - STORE_16(value, address & 0x0001FFFE, gba->video.vram); - gba->video.renderer->writeVRAM(gba->video.renderer, address & 0x0001FFFE); - } - } else { - LOAD_16(oldValue, address & 0x00017FFE, gba->video.vram); - if (value != oldValue) { - STORE_16(value, address & 0x00017FFE, gba->video.vram); - gba->video.renderer->writeVRAM(gba->video.renderer, address & 0x00017FFE); + if ((address & 0x0001FFFF) >= SIZE_VRAM) { + if ((address & (SIZE_VRAM | 0x00014000)) == SIZE_VRAM && (GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3)) { + mLOG(GBA_MEM, GAME_ERROR, "Bad VRAM Store16: 0x%08X", address); + break; } + address &= 0x00017FFE; + } + LOAD_16(oldValue, address & 0x0001FFFE, gba->video.vram); + if (value != oldValue) { + STORE_16(value, address & 0x0001FFFE, gba->video.vram); + gba->video.renderer->writeVRAM(gba->video.renderer, address & 0x0001FFFE); } break; case REGION_OAM: @@ -939,7 +948,6 @@ void GBAStore8(struct ARMCore* cpu, uint32_t address, int8_t value, int* cycleCo break; case REGION_VRAM: if ((address & 0x0001FFFF) >= ((GBARegisterDISPCNTGetMode(gba->memory.io[REG_DISPCNT >> 1]) >= 3) ? 0x00014000 : 0x00010000)) { - // TODO: check BG mode mLOG(GBA_MEM, GAME_ERROR, "Cannot Store8 to OBJ: 0x%08X", address); break; }