]> Kevux Git Server - rit/commitdiff
transport: plug leaks in transport_color_config()
authorJeff King <peff@peff.net>
Sat, 14 Mar 2026 16:08:14 +0000 (12:08 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 16 Mar 2026 20:31:48 +0000 (13:31 -0700)
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
transport.c

index e305d6bd228b456daace3e40aa8521aba2077534..7985b42a7431ca2068547f5e912f75c32abc7dd5 100644 (file)
@@ -47,21 +47,21 @@ static int transport_color_config(void)
                "color.transport.reset",
                "color.transport.rejected"
        }, *key = "color.transport";
-       char *value;
+       const char *value;
        static int initialized;
 
        if (initialized)
                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)