[PATCH 3/4] remote: fix leak in branch_get_push_1() with invalid "simple" config
- From
Jeff King <peff@peff.net>
- Date
- Jan 19, 2026, 05:22 UTC
- Message-ID
- <20260119052208.GC1991523@coredump.intra.peff.net>
- 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