Re: [PATCH 14/28] http: fix leak of http_object_request struct
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Sep 26, 2024, 13:50 UTC
- Message-ID
- <ZvVmmV0nYq_cEnj3@pks.im>
- In-Reply-To
- <20240924220109.GN1143820@coredump.intra.peff.net>
On Tue, Sep 24, 2024 at 06:01:09PM -0400, Jeff King wrote:
> The new_http_object_request() function allocates a struct on the heap, > along with some fields inside the struct. But the matching function to > clean it up, release_http_object_request(), only frees the interior > fields without freeing the struct itself, causing a leak.
Oh yeah, I remember staring at this code and being completely confused as to how this all works.
Show 33 quoted lines
> diff --git a/http.c b/http.c
> index cc136408c0..d0242ffb50 100644
> --- a/http.c
> +++ b/http.c
> @@ -2816,15 +2816,17 @@ int finish_http_object_request(struct http_object_request *freq)
> return freq->rename;
> }
>
> -void abort_http_object_request(struct http_object_request *freq)
> +void abort_http_object_request(struct http_object_request **freq_p)
> {
> + struct http_object_request *freq = *freq_p;
> unlink_or_warn(freq->tmpfile.buf);
>
> - release_http_object_request(freq);
> + release_http_object_request(freq_p);
> }
>
> -void release_http_object_request(struct http_object_request *freq)
> +void release_http_object_request(struct http_object_request **freq_p)
> {
> + struct http_object_request *freq = *freq_p;
> if (freq->localfile != -1) {
> close(freq->localfile);
> freq->localfile = -1;
> @@ -2838,4 +2840,7 @@ void release_http_object_request(struct http_object_request *freq)
> }
> curl_slist_free_all(freq->headers);
> strbuf_release(&freq->tmpfile);
> +
> + free(freq);
> + *freq_p = NULL;
> }Okay, looks simple enough. But I found the whole code in "http.c" to be quite... elusive, so I had a hard time finding my way.
Patrick