[PATCH v2 16/20] remote: fix leaks when matching refspecs
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 22, 2024, 09:17 UTC
- Message-ID
- <440b3d993724c465971628e9548c4937a7cd0833.1724315484.git.ps@pks.im>
- In-Reply-To
- <cover.1724315484.git.ps@pks.im>
In `match_explicit()`, we try to match a source ref with a destination ref according to a refspec item. This matching sometimes requires us to allocate a new source spec so that it looks like we expect. And while we in some end up assigning this allocated ref as `peer_ref`, which hands over ownership of it to the caller, in other cases we don't. We neither free it though, causing a memory leak.
Fix the leak by creating a common exit path where we can easily free the source ref in case it is allocated and hasn't been handed over to the caller.
Signed-off-by: Patrick Steinhardt <ps@pks.im> --- remote.c | 43 +++++++++++++++++++++++++++++-------------- t/t5505-remote.sh | 1 + 2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/remote.c b/remote.c index 2c52119bbb2..6ea81f9665b 100644 --- a/remote.c +++ b/remote.c @@ -1344,18 +1344,21 @@ static int match_explicit(struct ref *src, struct ref *dst, struct ref ***dst_tail, struct refspec_item *rs) { - struct ref *matched_src, *matched_dst; - int allocated_src; + struct ref *matched_src = NULL, *matched_dst = NULL; + int allocated_src = 0, ret; const char *dst_value = rs->dst; char *dst_guess; - if (rs->pattern || rs->matching || rs->negative) - return 0; + if (rs->pattern || rs->matching || rs->negative) { + ret = 0; + goto out; + } - matched_src = matched_dst = NULL; - if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) - return -1; + if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) { + ret = -1; + goto out; + } if (!dst_value) { int flag; @@ -1394,18 +1397,30 @@ static int match_explicit(struct ref *src, struct ref *dst, dst_value); break; } - if (!matched_dst) - return -1; - if (matched_dst->peer_ref) - return error(_("dst ref %s receives from more than one src"), - matched_dst->name); - else { + + if (!matched_dst) { + ret = -1; + goto out; + } + + if (matched_dst->peer_ref) { + ret = error(_("dst ref %s receives from more than one src"), + matched_dst->name); + goto out; + } else { matched_dst->peer_ref = allocated_src ? matched_src : copy_ref(matched_src); matched_dst->force = rs->force; + matched_src = NULL; } - return 0; + + ret = 0; + +out: + if (allocated_src) + free_one_ref(matched_src); + return ret; } static int match_explicit_refs(struct ref *src, struct ref *dst, diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 08424e878e1..532035933f3 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -2,6 +2,7 @@ test_description='git remote porcelain-ish' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh setup_repository () {
-- 2.46.0.164.g477ce5ccd6.dirty