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 double free crash #992

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Fix double free crash #992

merged 1 commit into from
Jun 5, 2024

Conversation

bmatherly
Copy link
Member

If the caller frees the returned object (which they should), then a double free error will occur because the encapsulated mlt_profile will be freed by the Profile and by the origional service that owned it.

If the caller frees the returned object (which they should), then
a double free error will occur because the encapsulated mlt_profile
will be freed by the Profile and by the origional service that owned it.
@ddennedy
Copy link
Member

ddennedy commented Jun 5, 2024

Where is this other free? Services do not own the profile.

@ddennedy
Copy link
Member

ddennedy commented Jun 5, 2024

Mlt::FilteredProducer::detach() constructs a Profile whose destructor frees the mlt_profile via mlt_profile_close() but the Mlt::Profile is not required due to a Mlt::Producer constructor that takes a mlt_profile:

--- src/mlt++/MltFilteredProducer.cpp
+++ src/mlt++/MltFilteredProducer.cpp
@@ -66,8 +66,7 @@ int FilteredProducer::detach(Filter &filter)
             Service *consumer = it->consumer();
             if (consumer->is_valid())
                 consumer->connect_producer(*producer);
-            Profile p(get_profile());
-            Producer dummy(p, "colour");
+            Producer dummy(get_profile(), "colour");
             dummy.connect_producer(*it);
             if (last->get_service() == it->get_service()) {
                 delete last;

@ddennedy ddennedy added this to the v7.26.0 milestone Jun 5, 2024
@ddennedy ddennedy merged commit 7a6cc5d into master Jun 5, 2024
12 checks passed
@ddennedy ddennedy deleted the profile_double_free branch June 5, 2024 03:35
ddennedy added a commit that referenced this pull request Jun 5, 2024
@bmatherly
Copy link
Member Author

Where is this other free? Services do not own the profile.

Presumably, whoever owns the mlt_profile that was passed to the constructor will either still try to use it (access freed memory) or free it (double free).

I wonder if we should deprecate Service::profile() to discourage its use. Mlt::Properties does not follow the same pattern as other Mlt classes. Other classes that take an mlt_properties based object in their constructor work properly because mlt_properties are reference counted.

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