From: Jeff King <peff@peff•net>
To: git@vger•kernel.org
Cc: Harald Nordgren <haraldnordgren@gmail•com>
Subject: [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config
Date: Mon, 19 Jan 2026 00:22:08 -0500 [thread overview]
Message-ID: <20260119052208.GC1991523@coredump.intra.peff.net> (raw)
In-Reply-To: <20260119051858.GA1991308@coredump.intra.peff.net>
Most of the code paths in branch_get_push_1() allocate a string for the
@{push} value. We then return the result, which is stored in a "struct
branch", so the value is not leaked.
But there's one path that does leak: when we are in the "simple" push
mode, we have to check that the @{push} value matches what we'd get for
@{upstream}. If it doesn't, we return an error, but forget to free the
@{push} value we computed.
Curiously, the existing tests don't trigger this with LSan, even though
they do exercise the code path. As far as I can tell, it should be
triggered via:
git -c push.default=simple \
-c branch.foo.remote=origin \
-c branch.foo.merge=refs/heads/not-foo \
rev-parse foo@{push}
which will complain that the upstream ("not-foo") does not match the
push destination ("foo"). We do die() shortly after this, but not until
after returning from branch_get_push_1(), which is where the leak
happens.
So it seems like a false negative in LSan. However, I can trigger it
reliably by printing the @{push} value using for-each-ref. This takes a
little more setup (because we need "foo" to actually exist to iterate
over it with for-each-ref), but we can piggy-back on the existing repo
config in t6300.
Signed-off-by: Jeff King <peff@peff•net>
---
remote.c | 4 +++-
t/for-each-ref-tests.sh | 9 +++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 5de9619bc7..e191b0ff6e 100644
--- a/remote.c
+++ b/remote.c
@@ -1938,9 +1938,11 @@ static const char *branch_get_push_1(struct repository *repo,
cur = tracking_for_push_dest(remote, branch->refname, err);
if (!cur)
return NULL;
- if (strcmp(cur, up))
+ if (strcmp(cur, up)) {
+ free(cur);
return error_buf(err,
_("cannot resolve 'simple' push to a single destination"));
+ }
return cur;
}
}
diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh
index 4593be5fd5..bd2d45c971 100644
--- a/t/for-each-ref-tests.sh
+++ b/t/for-each-ref-tests.sh
@@ -1744,6 +1744,15 @@ test_expect_success ':remotename and :remoteref' '
)
'
+test_expect_success '%(push) with an invalid push-simple config' '
+ echo "refs/heads/main " >expect &&
+ git -c push.default=simple \
+ -c remote.pushdefault=myfork \
+ for-each-ref \
+ --format="%(refname) %(push)" refs/heads/main >actual &&
+ test_cmp expect actual
+'
+
test_expect_success "${git_for_each_ref} --ignore-case ignores case" '
${git_for_each_ref} --format="%(refname)" refs/heads/MAIN >actual &&
test_must_be_empty actual &&
--
2.53.0.rc0.338.g08aa8a9473
next prev parent reply other threads:[~2026-01-19 5:22 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-19 5:18 [PATCH 0/4] memory leaks in remote.c Jeff King
2026-01-19 5:19 ` [PATCH 1/4] remote: return non-const pointer from error_buf() Jeff King
2026-01-19 6:33 ` Patrick Steinhardt
2026-01-20 0:28 ` Junio C Hamano
2026-01-20 19:38 ` Jeff King
2026-01-20 20:18 ` Junio C Hamano
2026-01-19 5:20 ` [PATCH 2/4] remote: drop const return of tracking_for_push_dest() Jeff King
2026-01-19 6:34 ` Patrick Steinhardt
2026-01-19 5:22 ` Jeff King [this message]
2026-01-19 6:34 ` [PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config Patrick Steinhardt
2026-01-19 5:23 ` [PATCH 4/4] remote: always allocate branch.push_tracking_ref Jeff King
2026-01-19 6:34 ` Patrick Steinhardt
2026-01-19 15:04 ` Triangular workflow Harald Nordgren
2026-01-20 19:40 ` Jeff King
2026-01-20 0:31 ` [PATCH 0/4] memory leaks in remote.c Junio C Hamano
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=20260119052208.GC1991523@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=git@vger$(echo .)kernel.org \
--cc=haraldnordgren@gmail$(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