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

Fix use-after-free in EditorHelp #95307

Merged
merged 1 commit into from
Aug 9, 2024

Conversation

mihe
Copy link
Contributor

@mihe mihe commented Aug 9, 2024

Fixes #95306.

This adds a zeroing out of the EditorHelp::doc variable after it's been freed, to prevent any use-after-free from happening in places like EditorHelp::remove_class.

While this does fix the issue at hand, there are quite a lot of places in EditorHelp that use this doc variable without checking if it's nullptr first, so I'm not sure if there's a bigger problem lurking here or not.

@akien-mga
Copy link
Member

While this does fix the issue at hand, there are quite a lot of places in EditorHelp that use this doc variable without checking if it's nullptr first, so I'm not sure if there's a bigger problem lurking here or not.

doc is only deleted in cleanup_doc which is only called from the EditorNode destructor, so at worst other issues might also be user-after-free or crashes on exit.

I see there's a TODO related to this:

// TODO: this is sometimes used directly as doc->something, other times as EditorHelp::get_doc_data(), which is thread-safe.
// Might this be a problem?
DocTools *EditorHelp::doc = nullptr;

Would probably be worth cleaning this up in the 4.4 cycle.

@akien-mga akien-mga merged commit d7c8a9f into godotengine:master Aug 9, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@mihe mihe deleted the editor-help-use-after-free branch August 9, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use-after-free when closing editor while using GDExtension that has registered classes
3 participants