git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 3/4] send-pack: fix memory leak around duplicate refs

From
Karthik Nayak <karthik.188@gmail.com>
Date
May 19, 2025, 09:58 UTC
Message-ID
<20250519-501-update-git-fetch-1-to-use-partial-transactions-v3-3-6cdfd4f769b9@gmail.com>
In-Reply-To
<20250519-501-update-git-fetch-1-to-use-partial-transactions-v3-0-6cdfd4f769b9@gmail.com>

The 'git-send-pack(1)' allows users to push objects to a remote repository and explicitly list the references to be pushed. The status of each reference pushed is captured into a list mapped by refname.

If a reference fails to be updated, its error message is captured in the `ref->remote_status` field. While the command allows duplicate ref inputs, the list doesn't accommodate this behavior as a particular refname is linked to a single `struct ref*` element. So if the user inputs a reference twice like:

  git send-pack remote.git A:foo B:foo

where the user is trying to update the same reference 'foo' twice and the reference fails to be updated, we first fill `ref->remote_status` with error message for the input 'A:foo' then we override the same field with the error message for 'B:foo'. This override happens without first free'ing the previous value. Fix this leak.

The current tests already incorporate the above example, but in the test 'A:foo' succeeds while 'B:foo' fails, meaning that the memory leak isn't triggered. Add a new test with multiple duplicates.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 send-pack.c                | 7 +++++++
 t/t5408-send-pack-stdin.sh | 6 ++++++
 2 files changed, 13 insertions(+)
diff --git a/send-pack.c b/send-pack.c
index 5005689cb5..4cd41a64ce 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -260,6 +260,13 @@ static int receive_status(struct repository *r,
 				refname);
 			continue;
 		}
+
+		/*
+		 * Clients sending duplicate refs can cause the same value
+		 * to be overridden, causing a memory leak.
+		 */
+		free(hint->remote_status);
+
 		if (!strcmp(head, "ng")) {
 			hint->status = REF_STATUS_REMOTE_REJECT;
 			if (p)
diff --git a/t/t5408-send-pack-stdin.sh b/t/t5408-send-pack-stdin.sh
index 526a675045..45fb20179b 100755
--- a/t/t5408-send-pack-stdin.sh
+++ b/t/t5408-send-pack-stdin.sh
@@ -73,6 +73,12 @@ test_expect_success 'cmdline refs written in order' '
 	verify_push A foo
 '
 
+test_expect_success 'cmdline refs with multiple duplicates' '
+	clear_remote &&
+	test_must_fail git send-pack remote.git A:foo B:foo C:foo &&
+	verify_push A foo
+'
+
 test_expect_success '--stdin refs come after cmdline' '
 	clear_remote &&
 	echo A:foo >input &&
-- 
2.49.0
Previous: Karthik NayakNext: Karthik Nayak
Message 4 of 11 in “fetch/receive: use batched reference updates”
  1. 0/4 fetch/receive: use batched reference updatesKarthik Nayak, May 19, 2025
  2. 1/4 refs: add function to translate errors to stringsKarthik Nayak, May 19, 2025
  3. 2/4 fetch: use batched reference updatesKarthik Nayak, May 19, 2025
  4. 3/4 send-pack: fix memory leak around duplicate refsKarthik Nayak, May 19, 2025
  5. 4/4 receive-pack: use batched reference updatesKarthik Nayak, May 19, 2025
  6. Junio C HamanoMay 19, 2025
  7. Karthik NayakMay 20, 2025
  8. Junio C HamanoMay 21, 2025
  9. Jeff KingMay 22, 2025
  10. Karthik NayakMay 22, 2025
  11. Jeff KingMay 22, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.