Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PCEHawk can crash due to invalid DMA resulting in an out of range read #1363

Closed
NarryG opened this issue Nov 4, 2018 · 5 comments
Closed
Assignees
Labels
Bug (GoogleCode) Grandfathered label from GoogleCode. Use the other bug labels. Core: PCEHawk (Alt.) PC Engine (PCE) / TurboGrafx-16 (TG-16) / SuperGrafx core Priority: Low

Comments

@NarryG
Copy link
Contributor

NarryG commented Nov 4, 2018

image

Call stack (old version of Bizhawk but the code in question hasn't changed)

BizHawk.Emulation.Cores.dll!BizHawk.Emulation.Cores.PCEngine.VDC.UpdatePatternData(ushort addr) Line 271	C#
 	BizHawk.Emulation.Cores.dll!BizHawk.Emulation.Cores.PCEngine.VDC.RunDmaForScanline() Line 231	C#
 	BizHawk.Emulation.Cores.dll!BizHawk.Emulation.Cores.PCEngine.VDC.ExecFrame(bool render) Line 95	C#
	BizHawk.Emulation.Cores.dll!BizHawk.Emulation.Cores.PCEngine.PCEngine.FrameAdvance(BizHawk.Emulation.Common.IController controller, bool render, bool rendersound) Line 28	C#

Note: To cause this, I had to override read-only on the 21-bit bus and then I fuzzed it. That doesn't mean a buggy game couldn't cause the behavior though.

@zeromus
Copy link
Contributor

zeromus commented Nov 30, 2018

one of you guys ought to be able to fix this pretty easy...

@zeromus zeromus added Bug (GoogleCode) Grandfathered label from GoogleCode. Use the other bug labels. Priority: Low Core: PCEHawk (Alt.) PC Engine (PCE) / TurboGrafx-16 (TG-16) / SuperGrafx core labels Nov 30, 2018
@Asnivor
Copy link
Contributor

Asnivor commented Nov 30, 2018

Havent got time right now, but shouldn't this be something as simple as:

public void UpdatePatternData(ushort addr)
{
	if (addr >= 0x8000)
		return;
..........

?

@zeromus
Copy link
Contributor

zeromus commented Nov 30, 2018

More likely it needs to be masked down by 1 bit

@Asnivor
Copy link
Contributor

Asnivor commented Nov 30, 2018

Ahh ok. Thats a much better idea as its almost certainly what would happen on real hardware.

Asnivor added a commit that referenced this issue Dec 3, 2018
Asnivor added a commit that referenced this issue Dec 4, 2018
PCEHawk - DESR enforce 16bit register size - #1363
@Asnivor
Copy link
Contributor

Asnivor commented Dec 4, 2018

This should now be fixed in the latest dev build.

@NarryG If you do happen to be able to break this in the same way as before, please reopen this issue.

@Asnivor Asnivor closed this as completed Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug (GoogleCode) Grandfathered label from GoogleCode. Use the other bug labels. Core: PCEHawk (Alt.) PC Engine (PCE) / TurboGrafx-16 (TG-16) / SuperGrafx core Priority: Low
Projects
None yet
Development

No branches or pull requests

5 participants