From: Jeff King <peff@peff•net>
To: Junio C Hamano <gitster@pobox•com>
Cc: Andrew Au <cshung@gmail•com>, git@vger•kernel.org
Subject: Re: [PATCH v4] transport-helper, connect: use clean_on_exit to reap children on abnormal exit
Date: Sat, 14 Mar 2026 12:08:14 -0400 [thread overview]
Message-ID: <20260314160814.GA918806@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqsea4aen2.fsf@gitster.g>
On Thu, Mar 12, 2026 at 03:04:17PM -0700, Junio C Hamano wrote:
> Thanks, queued.
Curiously this patch seems to cause a failure in one of the CI leak
jobs, but I don't think it's the culprit. See below for a fix and
explanation.
I don't know if you want to apply it separately (since it's really a
totally different topic) or on top (since it is only the application of
Andrew's patch which lets us find the problem).
-- >8 --
Subject: [PATCH] transport: plug leaks in transport_color_config()
We retrieve config values with repo_config_get_string(), which will
allocate a new copy of the string for us. But we don't hold on to those
strings, since they are just fed to git_config_colorbool() and
color_parse(). But nor do we free them, which means they leak.
We can fix this by using the "_tmp" form of repo_config_get_string(),
which just hands us a pointer directly to the internal storage. This is
OK for our purposes, since we don't need it to last for longer than our
parsing calls.
Two interesting side notes here:
1. Many types already have a repo_config_get_X() variant that handles
this for us (e.g., repo_config_get_bool()). But neither colorbools
nor colors themselves have such helpers. We might think about
adding them, but converting all callers is a larger task, and out
of scope for this fix.
2. As far as I can tell, this leak has been there since 960786e761
(push: colorize errors, 2018-04-21), but wasn't detected by LSan in
our test suite. It started triggering when we applied dd3693eb08
(transport-helper, connect: use clean_on_exit to reap children on
abnormal exit, 2026-03-12) which is mostly unrelated.
Even weirder, it seems to trigger only with clang (and not gcc),
and only with GIT_TEST_DEFAULT_REF_FORMAT=reftable. So I think this
is another odd case where the pointers happened to be hanging
around in stack memory, but changing the pattern of function calls
in nearby code was enough for them to be incidentally overwritten.
Signed-off-by: Jeff King <peff@peff•net>
---
transport.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index 107f4fa5dc..2fb4767821 100644
--- a/transport.c
+++ b/transport.c
@@ -54,14 +54,14 @@ static int transport_color_config(void)
return 0;
initialized = 1;
- if (!repo_config_get_string(the_repository, key, &value))
+ if (!repo_config_get_string_tmp(the_repository, key, &value))
transport_use_color = git_config_colorbool(key, value);
if (!want_color_stderr(transport_use_color))
return 0;
for (size_t i = 0; i < ARRAY_SIZE(keys); i++)
- if (!repo_config_get_string(the_repository, keys[i], &value)) {
+ if (!repo_config_get_string_tmp(the_repository, keys[i], &value)) {
if (!value)
return config_error_nonbool(keys[i]);
if (color_parse(value, transport_colors[i]) < 0)
--
2.53.0.887.g3d5d06adec
next prev parent reply other threads:[~2026-03-14 16:08 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-23 16:51 [PATCH 0/1] Fix zombie children when git is PID 1 in containers Andrew Au
2026-02-23 16:51 ` [PATCH 1/1] transport-helper, connect: add atexit handler to reap children on abnormal exit Andrew Au
2026-02-23 17:14 ` Kristoffer Haugsbakk
2026-02-23 18:12 ` Andrew Au
2026-03-11 14:20 ` [PATCH v2] " Andrew Au
2026-03-11 17:58 ` Junio C Hamano
2026-03-11 18:19 ` Andrew Au
2026-03-11 19:38 ` Junio C Hamano
2026-03-11 18:42 ` Jeff King
2026-03-12 19:55 ` [PATCH v3] transport-helper, connect: use clean_on_exit " Andrew Au
2026-03-12 20:40 ` Jeff King
2026-03-12 20:41 ` Jeff King
2026-03-12 20:49 ` Junio C Hamano
2026-03-12 21:49 ` [PATCH v4] " Andrew Au
2026-03-12 22:04 ` Junio C Hamano
2026-03-14 16:08 ` Jeff King [this message]
2026-03-14 17:24 ` Junio C Hamano
2026-03-16 20:31 ` Junio C Hamano
2026-03-16 21:19 ` Jeff King
2026-03-16 21:24 ` Junio C Hamano
2026-03-11 21:15 ` [PATCH 0/1] Fix zombie children when git is PID 1 in containers brian m. carlson
2026-03-12 19:40 ` Andrew Au
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=20260314160814.GA918806@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=cshung@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox