Skip to content

libretro: detect and communicate buffer overflow on save state serialization#2089

Merged
flyinghead merged 2 commits intoflyinghead:masterfrom
jlansing:core_savestate_fix1
Oct 21, 2025
Merged

libretro: detect and communicate buffer overflow on save state serialization#2089
flyinghead merged 2 commits intoflyinghead:masterfrom
jlansing:core_savestate_fix1

Conversation

@jlansing
Copy link
Contributor

As discussed in Issue #1903, the libretro core uses dynamic sizing for save states. If the size requirements increase over time by the core, then the RetroArch front end will disregard it and only supply the initial lower size provided in retro_serialize_size(). This creates a scenario where a buffer is passed to retro_serialize() that is not big enough to store all of the necessary data, and will result in a buffer overflow. In these cases, though it appears to succeed in creating the save state, the file itself is incomplete and cannot be successfully loaded. It's possible the overflow can cause other issues but I saw no evidence of this outside of the incomplete save state file.

This PR aims to improve the situation by:

  1. Detecting and preventing buffer overflow by throwing within the serializer (similar to how deserialization already works).
  2. Catching this in libretro and communicating to the front end that serialization failed.

This does not fix the underlying issue, but correctly communicates to the front end and the user* that creating the save state failed.

*In testing, it looks like the current version of RA does not actually show an error to the user that a save state failed to save, though it does not say it was successful either like it did before. I plan on filing a PR to make this more visible on the RA front end as well.

core/serialize.h Outdated
public:
Exception(const char *msg) : std::runtime_error(msg) {}
};

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to move the existing Deserializer::Exception to the base class SerializeBase so it can be used by both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, just pushed that change.

std::vector<int> data(10);
Serializer ser(buffer.data(), buffer.size());
ASSERT_THROW(ser.serialize(data.data(), data.size()), Serializer::Exception);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

@flyinghead flyinghead merged commit bf2bd7e into flyinghead:master Oct 21, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants