From: Patrick Steinhardt <ps@pks.im>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 15/28] http: call git_inflate_end() when releasing http_object_request
Date: Thu, 26 Sep 2024 15:50:20 +0200 [thread overview]
Message-ID: <ZvVmnCJNRY_V0a8a@pks.im> (raw)
In-Reply-To: <20240924220213.GO1143820@coredump.intra.peff.net>
On Tue, Sep 24, 2024 at 06:02:13PM -0400, Jeff King wrote:
> In new_http_object_request(), we initialize the zlib stream with
> git_inflate_init(). We must have a matching git_inflate_end() to avoid
> leaking any memory allocated by zlib.
>
> In most cases this happens in finish_http_object_request(), but we don't
> always get there. If we abort a request mid-stream, then we may clean it
> up without hitting that function.
>
> We can't just add a git_inflate_end() call to the release function,
> though. That would double-free the cases that did actually finish.
> Instead, we'll move the call from the finish function to the release
> function. This does delay it for the cases that do finish, but I don't
> think it matters. We should have already reached Z_STREAM_END (and
> complain if we didn't), and we do not record any status code from
> git_inflate_end().
I had to read this paragraph multiple times to understand it, as I
wondered why you did end up adding it to `release_http_object_request()`
even though the paragraph claims that you cannot. But what you say is
that you must _move_ the call, not add it, and that's what the patch
does.
So yeah, that does make sense.
Patrick
next prev parent reply other threads:[~2024-09-26 13:50 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-24 21:49 [PATCH 0/28] leak fixes for http fetch/push Jeff King
2024-09-24 21:50 ` [PATCH 01/28] http-fetch: clear leaking git-index-pack(1) arguments Jeff King
2024-09-24 21:50 ` [PATCH 02/28] shallow: fix leak when unregistering last shallow root Jeff King
2024-09-24 21:51 ` [PATCH 03/28] fetch-pack: fix leaking sought refs Jeff King
2024-09-25 17:17 ` René Scharfe
2024-09-26 11:52 ` Patrick Steinhardt
2024-09-24 21:51 ` [PATCH 04/28] connect: clear child process before freeing in diagnostic mode Jeff King
2024-09-26 13:49 ` Patrick Steinhardt
2024-09-24 21:52 ` [PATCH 05/28] fetch-pack: free object filter before exiting Jeff King
2024-09-26 13:49 ` Patrick Steinhardt
2024-09-24 21:52 ` [PATCH 06/28] fetch-pack, send-pack: clean up shallow oid array Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-27 3:45 ` Jeff King
2024-09-24 21:54 ` [PATCH 07/28] commit: avoid leaking already-saved buffer Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-24 21:55 ` [PATCH 08/28] send-pack: free cas options before exit Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-27 3:47 ` Jeff King
2024-09-24 21:56 ` [PATCH 09/28] transport-helper: fix strbuf leak in push_refs_with_push() Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-27 3:49 ` Jeff King
2024-09-24 21:57 ` [PATCH 10/28] fetch: free "raw" string when shrinking refspec Jeff King
2024-09-24 21:58 ` [PATCH 11/28] fetch-pack: clear pack lockfiles list Jeff King
2024-09-24 21:58 ` [PATCH 12/28] transport-helper: fix leak of dummy refs_list Jeff King
2024-09-24 21:59 ` [PATCH 13/28] http: fix leak when redacting cookies from curl trace Jeff King
2024-09-24 22:01 ` [PATCH 14/28] http: fix leak of http_object_request struct Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-27 3:50 ` Jeff King
2024-09-24 22:02 ` [PATCH 15/28] http: call git_inflate_end() when releasing http_object_request Jeff King
2024-09-26 13:50 ` Patrick Steinhardt [this message]
2024-09-27 3:51 ` Jeff King
2024-09-24 22:02 ` [PATCH 16/28] http: stop leaking buffer in http_get_info_packs() Jeff King
2024-09-24 22:03 ` [PATCH 17/28] remote-curl: free HEAD ref with free_one_ref() Jeff King
2024-09-24 22:04 ` [PATCH 18/28] http-walker: free fake packed_git list Jeff King
2024-09-24 22:04 ` [PATCH 19/28] http-push: clear refspecs before exiting Jeff King
2024-09-24 22:04 ` [PATCH 20/28] http-push: free repo->url string Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-27 3:55 ` Jeff King
2024-09-24 22:05 ` [PATCH 21/28] http-push: free curl header lists Jeff King
2024-09-26 13:50 ` Patrick Steinhardt
2024-09-24 22:06 ` [PATCH 22/28] http-push: free transfer_request dest field Jeff King
2024-09-24 22:08 ` [PATCH 23/28] http-push: free transfer_request strbuf Jeff King
2024-09-24 22:09 ` [PATCH 24/28] http-push: free remote_ls_ctx.dentry_name Jeff King
2024-09-24 22:09 ` [PATCH 25/28] http-push: free xml_ctx.cdata after use Jeff King
2024-09-24 22:10 ` [PATCH 26/28] http-push: clean up objects list Jeff King
2024-09-24 22:11 ` [PATCH 27/28] http-push: clean up loose request when falling back to packed Jeff King
2024-09-24 22:12 ` [PATCH 28/28] http-push: clean up local_refs at exit Jeff King
2024-09-26 13:50 ` [PATCH 0/28] leak fixes for http fetch/push Patrick Steinhardt
2024-09-27 3:55 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZvVmnCJNRY_V0a8a@pks.im \
--to=ps@pks.im \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.