git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Junio C Hamano <gitster@pobox.com>,
	Karthik Nayak <karthik.188@gmail.com>, Jeff King <peff@peff.net>
Subject: [PATCH v3 03/21] transport-helper: fix leaking helper name
Date: Mon, 27 May 2024 13:45:56 +0200	[thread overview]
Message-ID: <9e946c1a835bbdc23fbc6773b714eb2175ae33f8.1716810168.git.ps@pks.im> (raw)
In-Reply-To: <cover.1716810168.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 4033 bytes --]

When initializing the transport helper in `transport_get()`, we
allocate the name of the helper. We neither end up transferring
ownership of the name, nor do we free it. The associated memory thus
leaks.

Fix this memory leak by freeing the string at the calling side in
`transport_get()`. `transport_helper_init()` now creates its own copy of
the string and thus can free it as required.

An alterantive way to fix this would be to transfer ownership of the
string passed into `transport_helper_init()`, which would avoid the call
to xstrdup(1). But it does make for a more surprising calling convention
as we do not typically transfer ownership of strings like this.

Mark now-passing tests as leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t0611-reftable-httpd.sh    | 1 +
 t/t5563-simple-http-auth.sh  | 1 +
 t/t5564-http-proxy.sh        | 1 +
 t/t5581-http-curl-verbose.sh | 1 +
 transport-helper.c           | 6 ++++--
 transport.c                  | 1 +
 6 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/t/t0611-reftable-httpd.sh b/t/t0611-reftable-httpd.sh
index 5e05b9c1f2..2805995cc8 100755
--- a/t/t0611-reftable-httpd.sh
+++ b/t/t0611-reftable-httpd.sh
@@ -2,6 +2,7 @@
 
 test_description='reftable HTTPD tests'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 
diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
index 5d5caa3f58..4af796de67 100755
--- a/t/t5563-simple-http-auth.sh
+++ b/t/t5563-simple-http-auth.sh
@@ -2,6 +2,7 @@
 
 test_description='test http auth header and credential helper interop'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 
diff --git a/t/t5564-http-proxy.sh b/t/t5564-http-proxy.sh
index 9da5134614..bb35b87071 100755
--- a/t/t5564-http-proxy.sh
+++ b/t/t5564-http-proxy.sh
@@ -2,6 +2,7 @@
 
 test_description="test fetching through http proxy"
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 
diff --git a/t/t5581-http-curl-verbose.sh b/t/t5581-http-curl-verbose.sh
index cded79c16b..724f610054 100755
--- a/t/t5581-http-curl-verbose.sh
+++ b/t/t5581-http-curl-verbose.sh
@@ -4,6 +4,7 @@ test_description='test GIT_CURL_VERBOSE'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd
diff --git a/transport-helper.c b/transport-helper.c
index 780fcaf529..9820947ab2 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -22,7 +22,7 @@
 static int debug;
 
 struct helper_data {
-	const char *name;
+	char *name;
 	struct child_process *helper;
 	FILE *out;
 	unsigned fetch : 1,
@@ -111,6 +111,7 @@ static void do_take_over(struct transport *transport)
 	data = (struct helper_data *)transport->data;
 	transport_take_over(transport, data->helper);
 	fclose(data->out);
+	free(data->name);
 	free(data);
 }
 
@@ -253,6 +254,7 @@ static int disconnect_helper(struct transport *transport)
 		close(data->helper->out);
 		fclose(data->out);
 		res = finish_command(data->helper);
+		FREE_AND_NULL(data->name);
 		FREE_AND_NULL(data->helper);
 	}
 	return res;
@@ -1297,7 +1299,7 @@ static struct transport_vtable vtable = {
 int transport_helper_init(struct transport *transport, const char *name)
 {
 	struct helper_data *data = xcalloc(1, sizeof(*data));
-	data->name = name;
+	data->name = xstrdup(name);
 
 	transport_check_allowed(name);
 
diff --git a/transport.c b/transport.c
index 0ad04b77fd..83ddea8fbc 100644
--- a/transport.c
+++ b/transport.c
@@ -1176,6 +1176,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		int len = external_specification_len(url);
 		char *handler = xmemdupz(url, len);
 		transport_helper_init(ret, handler);
+		free(handler);
 	}
 
 	if (ret->smart_options) {
-- 
2.45.1.246.gb9cfe4845c.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2024-05-27 11:46 UTC|newest]

Thread overview: 115+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-23 12:25 [PATCH 00/20] Various memory leak fixes Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 01/20] t: mark a bunch of tests as leak-free Patrick Steinhardt
2024-05-23 17:44   ` Junio C Hamano
2024-05-24  6:56     ` Patrick Steinhardt
2024-05-24 16:05       ` Junio C Hamano
2024-05-24 17:53         ` Junio C Hamano
2024-05-24 20:34   ` Karthik Nayak
2024-05-23 12:25 ` [PATCH 02/20] transport-helper: fix leaking helper name Patrick Steinhardt
2024-05-23 17:36   ` Junio C Hamano
2024-05-24 20:38   ` Karthik Nayak
2024-05-23 12:25 ` [PATCH 03/20] strbuf: fix leak when `appendwholeline()` fails with EOF Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 04/20] checkout: clarify memory ownership in `unique_tracking_name()` Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 05/20] http: refactor code to clarify memory ownership Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 06/20] config: clarify memory ownership in `git_config_pathname()` Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 07/20] diff: refactor code to clarify memory ownership of prefixes Patrick Steinhardt
2024-05-23 16:59   ` Eric Sunshine
2024-05-23 12:25 ` [PATCH 08/20] convert: refactor code to clarify ownership of check_roundtrip_encoding Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 09/20] builtin/log: stop using globals for log config Patrick Steinhardt
2024-05-23 12:25 ` [PATCH 10/20] builtin/log: stop using globals for format config Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 11/20] config: clarify memory ownership in `git_config_string()` Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 12/20] config: plug various memory leaks Patrick Steinhardt
2024-05-23 17:13   ` Junio C Hamano
2024-05-24  6:58     ` Patrick Steinhardt
2024-05-24  8:55       ` Patrick Steinhardt
2024-05-24 16:12         ` Junio C Hamano
2024-05-24 16:11       ` Junio C Hamano
2024-05-23 12:26 ` [PATCH 13/20] builtin/credential: clear credential before exit Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 14/20] commit-reach: fix memory leak in `ahead_behind()` Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 15/20] submodule: fix leaking memory for submodule entries Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 16/20] strvec: add functions to replace and remove strings Patrick Steinhardt
2024-05-23 17:09   ` Eric Sunshine
2024-05-24  6:56     ` Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 17/20] builtin/mv: refactor `add_slash()` to always return allocated strings Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 18/20] builtin/mv duplicate string list memory Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 19/20] builtin/mv: refactor to use `struct strvec` Patrick Steinhardt
2024-05-23 12:26 ` [PATCH 20/20] builtin/mv: fix leaks for submodule gitfile paths Patrick Steinhardt
2024-05-23 16:45 ` [PATCH 00/20] Various memory leak fixes Junio C Hamano
2024-05-24  6:56   ` Patrick Steinhardt
2024-05-24 10:03 ` [PATCH v2 00/21] " Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 01/21] ci: add missing dependency for TTY prereq Patrick Steinhardt
2024-05-24 16:31     ` Junio C Hamano
2024-05-24 10:03   ` [PATCH v2 02/21] t: mark a bunch of tests as leak-free Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 03/21] transport-helper: fix leaking helper name Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 04/21] strbuf: fix leak when `appendwholeline()` fails with EOF Patrick Steinhardt
2024-05-25  4:46     ` Jeff King
2024-05-27  6:44       ` Patrick Steinhardt
2024-05-29  9:16         ` Jeff King
2024-05-29 11:25           ` Patrick Steinhardt
2024-05-30  7:16             ` Jeff King
2024-05-24 10:03   ` [PATCH v2 05/21] checkout: clarify memory ownership in `unique_tracking_name()` Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 06/21] http: refactor code to clarify memory ownership Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 07/21] config: clarify memory ownership in `git_config_pathname()` Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 08/21] diff: refactor code to clarify memory ownership of prefixes Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 09/21] convert: refactor code to clarify ownership of check_roundtrip_encoding Patrick Steinhardt
2024-05-24 10:03   ` [PATCH v2 10/21] builtin/log: stop using globals for log config Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 11/21] builtin/log: stop using globals for format config Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 12/21] config: clarify memory ownership in `git_config_string()` Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 13/21] config: plug various memory leaks Patrick Steinhardt
2024-05-24 10:13     ` Patrick Steinhardt
2024-05-25  4:33     ` Jeff King
2024-05-27  6:46       ` Patrick Steinhardt
2024-05-29  9:20         ` Jeff King
2024-05-24 10:04   ` [PATCH v2 14/21] builtin/credential: clear credential before exit Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 15/21] commit-reach: fix memory leak in `ahead_behind()` Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 16/21] submodule: fix leaking memory for submodule entries Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 17/21] strvec: add functions to replace and remove strings Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 18/21] builtin/mv: refactor `add_slash()` to always return allocated strings Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 19/21] builtin/mv duplicate string list memory Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 20/21] builtin/mv: refactor to use `struct strvec` Patrick Steinhardt
2024-05-24 10:04   ` [PATCH v2 21/21] builtin/mv: fix leaks for submodule gitfile paths Patrick Steinhardt
2024-05-25  2:10   ` [PATCH v2 00/21] Various memory leak fixes Junio C Hamano
2024-05-27  6:44     ` Patrick Steinhardt
2024-05-27 17:38       ` Junio C Hamano
2024-05-27 18:02         ` Junio C Hamano
2024-05-28  5:09         ` Patrick Steinhardt
2024-05-29  8:25       ` Karthik Nayak
2024-05-27 11:45 ` [PATCH v3 " Patrick Steinhardt
2024-05-27 11:45   ` [PATCH v3 01/21] ci: add missing dependency for TTY prereq Patrick Steinhardt
2024-05-27 11:45   ` [PATCH v3 02/21] t: mark a bunch of tests as leak-free Patrick Steinhardt
2024-05-27 11:45   ` Patrick Steinhardt [this message]
2024-05-27 11:46   ` [PATCH v3 04/21] strbuf: fix leak when `appendwholeline()` fails with EOF Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 05/21] checkout: clarify memory ownership in `unique_tracking_name()` Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 06/21] http: refactor code to clarify memory ownership Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 07/21] config: clarify memory ownership in `git_config_pathname()` Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 08/21] diff: refactor code to clarify memory ownership of prefixes Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 09/21] convert: refactor code to clarify ownership of check_roundtrip_encoding Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 10/21] builtin/log: stop using globals for log config Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 11/21] builtin/log: stop using globals for format config Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 12/21] config: clarify memory ownership in `git_config_string()` Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 13/21] config: plug various memory leaks Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 14/21] builtin/credential: clear credential before exit Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 15/21] commit-reach: fix memory leak in `ahead_behind()` Patrick Steinhardt
2024-05-27 11:46   ` [PATCH v3 16/21] submodule: fix leaking memory for submodule entries Patrick Steinhardt
2024-05-27 11:47   ` [PATCH v3 17/21] strvec: add functions to replace and remove strings Patrick Steinhardt
2024-05-27 11:47   ` [PATCH v3 18/21] builtin/mv: refactor `add_slash()` to always return allocated strings Patrick Steinhardt
2024-05-27 11:47   ` [PATCH v3 19/21] builtin/mv duplicate string list memory Patrick Steinhardt
2024-05-27 11:47   ` [PATCH v3 20/21] builtin/mv: refactor to use `struct strvec` Patrick Steinhardt
2024-05-27 11:47   ` [PATCH v3 21/21] builtin/mv: fix leaks for submodule gitfile paths Patrick Steinhardt
2024-05-27 17:52   ` [PATCH v3 00/21] Various memory leak fixes Junio C Hamano
2024-05-30  6:38   ` [PATCH 0/5] add-ons for ps/leakfixes Jeff King
2024-05-30  6:39     ` [PATCH 1/5] t-strvec: use va_end() to match va_start() Jeff King
2024-05-30  6:39     ` [PATCH 2/5] t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULL Jeff King
2024-05-30  6:44     ` [PATCH 3/5] mv: move src_dir cleanup to end of cmd_mv() Jeff King
2024-05-30  7:04       ` Patrick Steinhardt
2024-05-30  7:21         ` Jeff King
2024-05-30  7:24           ` Patrick Steinhardt
2024-05-30  8:15             ` Jeff King
2024-05-30  8:19               ` Patrick Steinhardt
2024-05-30  8:28                 ` Jeff King
2024-05-30  6:45     ` [PATCH 4/5] mv: factor out empty src_dir removal Jeff King
2024-05-30  6:46     ` [PATCH 5/5] mv: replace src_dir with a strvec Jeff King
2024-05-30 15:36       ` Junio C Hamano
2024-05-31 11:12         ` Jeff King
2024-05-31 14:56           ` Junio C Hamano
2024-05-30  7:05     ` [PATCH 0/5] add-ons for ps/leakfixes Patrick Steinhardt

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9e946c1a835bbdc23fbc6773b714eb2175ae33f8.1716810168.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).