libretro / gambatte-libretro

Hard fork of Gambatte to the libretro API.
http://sourceforge.net/projects/gambatte/
GNU General Public License v2.0
105 stars 79 forks source link

Disable retro_get_memory_* before ROM is loaded #105

Closed ehmry closed 6 years ago

ehmry commented 6 years ago

If a frontend calls retro_get_memory_data or retro_get_memory_size before a ROM is loaded the uninitialized cartridge memory is accessed.

andres-asm commented 6 years ago

Hmmm... The frontend should be fixed then

On Sun, Jul 8, 2018, 7:20 AM Emery Hemingway notifications@github.com wrote:

If a frontend calls retro_get_memory_data or retro_get_memory_size before a ROM is loaded the uninitialized cartridge memory is accessed.

You can view, comment on, or merge this pull request online at:

https://github.com/libretro/gambatte-libretro/pull/105 Commit Summary

  • Disable retro_getmemory* before ROM is loaded

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/libretro/gambatte-libretro/pull/105, or mute the thread https://github.com/notifications/unsubscribe-auth/ABpC0Nf7uUuYUXMMmJaPI1maxqcqEsihks5uEfkAgaJpZM4VGtGg .

ehmry commented 6 years ago

Sure, but retro_get_memory_data should be safe to call anytime between retro_init and retro_deinit.

ghost commented 6 years ago

retro_get_memory_data or retro_get_memory_size are called when retro_load_game is done. by then, any rom loading routines should already be ready for retro_run().. if not, then something is wrong and retro_load_game should fail...

not sure if other platform do call these API in advance but thats how it is on windows/linux at least.

ehmry commented 6 years ago

I'm making a PR because I caught Gambatt segfaulting, I make no claims on when why and where retro_get_memory_data should be called.

andres-asm commented 6 years ago

But it's segfaulting in which frontend?

ehmry commented 6 years ago

Any frontend that calls retro_get_memory_data before retro_load_game.

andres-asm commented 6 years ago

Yes, but then that frontend is doing it wrong, that's why I am asking which frontend. How could a core know the memory layout before even loading a game?

ehmry commented 6 years ago

I have my own frontend. For the sake of not arguing, its behavior is wrong. Gambatte still makes an unsafe access to the ROM memory and crashes in previous versions of my frontend, and in future frontends written by other people who cannot find and documentation on when retro_get_memory_data is unsafe to call.

ehmry commented 6 years ago

Calling retro_get_memory_data calls Cartridge::savedata_ptr that calls romdata(), which just returns offsets of a pointer to heap memory, and there are no checks in-between to ensure that pointer isn't null.

https://github.com/libretro/gambatte-libretro/blob/d5af1d7c4b339deab9e9f6fa999b5af3a96ca842/libgambatte/src/mem/memptrs.h#L66