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

Locations may not persist dump+undumps? #89

Open
vext01 opened this issue Sep 10, 2024 · 1 comment
Open

Locations may not persist dump+undumps? #89

vext01 opened this issue Sep 10, 2024 · 1 comment
Assignees

Comments

@vext01
Copy link
Contributor

vext01 commented Sep 10, 2024

Lua has this notion of "dumping" (serialising to a binary string) functions when they are GCd. Later they can be "undumped" back into Protos. A Proto is where we store our yk locations array.

@Pavel-Durov knew about this is the "old" branch, and attempted a fix:
#32

When we upgraded the Lua version, that change wasn't carried over.

Since we don't dump/undump the yk locations, I'd expect uninitialised memory accesses. but @Pavel-Durov was saying that it fixes a use after free. I'm not sure why that would be.

If/when we want to revisit this, we should review the old branch change, because it looks to me like it creates new yk locations upon undump, leaving the old ones to leak(?).

@vext01 vext01 self-assigned this Sep 10, 2024
@ltratt ltratt changed the title Locations may not persist dump+undumps. Potential UB? Locations may not persist dump+undumps Sep 10, 2024
@ltratt ltratt changed the title Locations may not persist dump+undumps Locations may not persist dump+undumps? Sep 10, 2024
@ltratt
Copy link
Contributor

ltratt commented Sep 10, 2024

I've changed the title because a bug is a bug, and UB is a bogeyman we use to scare children :p

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

No branches or pull requests

2 participants