Skip to content

Fix mem leak when duphandle fails in curl_clone_obj - #22894

Closed
Sjord wants to merge 1 commit into
php:masterfrom
Sjord:curl-clone-fail-leak
Closed

Fix mem leak when duphandle fails in curl_clone_obj#22894
Sjord wants to merge 1 commit into
php:masterfrom
Sjord:curl-clone-fail-leak

Conversation

@Sjord

@Sjord Sjord commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Move init_curl_handle so that we don't leak memory if curl_easy_duphandle fails within curl_clone_obj.

init_curl_handle allocates things, and curl_free_obj frees that again. But curl_free_obj does nothing if ch->cp is not set. In curl_clone_obj, do the allocation only once we have a valid cp, so that curl_free_obj actually deallocates the memory again. If curl_easy_duphandle fails we have not allocated anything and nothing is cleaned up.

@Sjord
Sjord requested a review from adoy as a code owner July 26, 2026 19:22
@LamentXU123

Copy link
Copy Markdown
Member

Please rebase to master to pass the arm CI :)

Move init_curl_handle so that we don't leak memory if
curl_easy_duphandle fails within curl_clone_obj.

init_curl_handle allocates things, and curl_free_obj frees that again.
But curl_free_obj does nothing if ch->cp is not set. In curl_clone_obj,
do the allocation only once we have a valid cp, so that curl_free_obj
actually deallocates the memory again. If curl_easy_duphandle
fails we have not allocated anything and nothing is cleaned up.
@Sjord
Sjord force-pushed the curl-clone-fail-leak branch from d20129b to dd965a0 Compare July 27, 2026 09:27
@LamentXU123

Copy link
Copy Markdown
Member

This feels like more like a bug fix to me. @Sjord do you think this should be merged into stable versions? (i.e. 8.4 8.5)

@Sjord

Sjord commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Yes: #22899

@Sjord
Sjord marked this pull request as draft July 27, 2026 09:58
@Sjord

Sjord commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

This PR should probably not be merged, but #22899 instead.

@LamentXU123

Copy link
Copy Markdown
Member

superseded by #22899

@LamentXU123

Copy link
Copy Markdown
Member

Merged in 3163ac7. Thank you.
btw, feel free to ping me in your future PRs.

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.

2 participants