remote: fix leaks when matching refspecs

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Aug 22, 2024 at 11:17 UTC 5e9e04a0641a933fd2da62139795ef6fd322835a
2 files changed +30 -14
remote.c
+29 -14
@@ -1344,18 +1344,21 @@ static int match_explicit(struct ref *src, struct ref *dst,
1344 struct ref ***dst_tail,
1345 struct refspec_item *rs)
1346 {
1347 - struct ref *matched_src, *matched_dst;
1348 - int allocated_src;
1347 + struct ref *matched_src = NULL, *matched_dst = NULL;
1348 + int allocated_src = 0, ret;
1349
1350 const char *dst_value = rs->dst;
1351 char *dst_guess;
1352
1353 - if (rs->pattern || rs->matching || rs->negative)
1354 - return 0;
1353 + if (rs->pattern || rs->matching || rs->negative) {
1354 + ret = 0;
1355 + goto out;
1356 + }
1357
1356 - matched_src = matched_dst = NULL;
1357 - if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)
1358 - return -1;
1358 + if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) {
1359 + ret = -1;
1360 + goto out;
1361 + }
1362
1363 if (!dst_value) {
1364 int flag;
@@ -1394,18 +1397,30 @@ static int match_explicit(struct ref *src, struct ref *dst,
1397 dst_value);
1398 break;
1399 }
1397 - if (!matched_dst)
1398 - return -1;
1399 - if (matched_dst->peer_ref)
1400 - return error(_("dst ref %s receives from more than one src"),
1401 - matched_dst->name);
1402 - else {
1400 +
1401 + if (!matched_dst) {
1402 + ret = -1;
1403 + goto out;
1404 + }
1405 +
1406 + if (matched_dst->peer_ref) {
1407 + ret = error(_("dst ref %s receives from more than one src"),
1408 + matched_dst->name);
1409 + goto out;
1410 + } else {
1411 matched_dst->peer_ref = allocated_src ?
1412 matched_src :
1413 copy_ref(matched_src);
1414 matched_dst->force = rs->force;
1415 + matched_src = NULL;
1416 }
1408 - return 0;
1417 +
1418 + ret = 0;
1419 +
1420 +out:
1421 + if (allocated_src)
1422 + free_one_ref(matched_src);
1423 + return ret;
1424 }
1425
1426 static int match_explicit_refs(struct ref *src, struct ref *dst,
t/t5505-remote.sh
+1
@@ -2,6 +2,7 @@
2
3 test_description='git remote porcelain-ish'
4
5 +TEST_PASSES_SANITIZE_LEAK=true
6 . ./test-lib.sh
7
8 setup_repository () {