diff mbox series

[v3,14/15] upload-pack: fix a memory leak in create_pack_file()

Message ID patch-v3-14.15-c1436269c64-20220727T230800Z-avarab@gmail.com (mailing list archive)
State Accepted
Commit c68d5dbc945347359f8a1431740e4e9ad9d99efc
Headers show
Series leak test: add "check" test mode, mark leak-free tests | expand

Commit Message

Ævar Arnfjörð Bjarmason July 27, 2022, 11:13 p.m. UTC
Fix a memory leak that's been reported by some versions of "gcc" since
"output_state" became malloc'd in 55a9651d26a (upload-pack.c: increase
output buffer size, 2021-12-14).

In e75d2f7f734 (revisions API: have release_revisions() release
"filter", 2022-04-13) it was correctly marked as leak-free, the only
path through this function that doesn't reach the free(output_state)
is if we "goto fail", and that will invoke "die()".

Such leaks are not included with SANITIZE=leak (but e.g. valgrind will
still report them), but under some gcc optimization (I have not been
able to reproduce it with "clang") we'll report a leak here
anyway. E.g. gcc v12 with "-O2" and above will trigger it, but not
clang v13 with any "-On".

The GitHub CI would also run into this leak if the "linux-leaks" job
was made to run with "GIT_TEST_SANITIZE_LEAK_LOG=true".

See [1] for a past case where gcc had similar trouble analyzing leaks
involving a die() invocation in the function.

1. https://lore.kernel.org/git/patch-v3-5.6-9a44204c4c9-20211022T175227Z-avarab@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t1060-object-corruption.sh | 1 +
 upload-pack.c                | 1 +
 2 files changed, 2 insertions(+)
diff mbox series

Patch

diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh
index e8a58b15897..5b8e47e346c 100755
--- a/t/t1060-object-corruption.sh
+++ b/t/t1060-object-corruption.sh
@@ -2,6 +2,7 @@ 
 
 test_description='see how we handle various forms of corruption'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # convert "1234abcd" to ".git/objects/12/34abcd"
diff --git a/upload-pack.c b/upload-pack.c
index 09f48317b02..b217a1f469e 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -455,6 +455,7 @@  static void create_pack_file(struct upload_pack_data *pack_data,
 	return;
 
  fail:
+	free(output_state);
 	send_client_data(3, abort_msg, sizeof(abort_msg),
 			 pack_data->use_sideband);
 	die("git upload-pack: %s", abort_msg);