From 7eb6f02c554b2e41f4b33152163868a84a0afa85 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:46 +0200 Subject: builtin/push: fix leaking refspec query result When appending a refspec via `refspec_append_mapped()` we leak the result of `query_refspecs()`. The overall logic around refspec queries is quite weird, as callers are expected to either set the `src` or `dst` pointers, and then the (allocated) result will be in the respective other struct member. As we have the `src` member set, plugging the memory leak is thus as easy as just freeing the `dst` member. While at it, use designated initializers to initialize the structure. This leak was exposed by t5516, but fixing it is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/push.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'builtin') diff --git a/builtin/push.c b/builtin/push.c index 7a67398124..0b123eb9c1 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -72,13 +72,15 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref, const char *branch_name; if (remote->push.nr) { - struct refspec_item query; - memset(&query, 0, sizeof(struct refspec_item)); - query.src = matched->name; + struct refspec_item query = { + .src = matched->name, + }; + if (!query_refspecs(&remote->push, &query) && query.dst) { refspec_appendf(refspec, "%s%s:%s", query.force ? "+" : "", query.src, query.dst); + free(query.dst); return; } } -- cgit v1.3 From 1e8cb17ac5c0e7871e15dd40bacd617f25a54763 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:08:56 +0200 Subject: builtin/submodule--helper: fix leaking refs on push-check In the push-check subcommand of the submodule helper we acquire a list of local refs, but never free that list. Fix this memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 ++ t/t5531-deep-submodule-push.sh | 1 + 2 files changed, 3 insertions(+) (limited to 'builtin') diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 85fb23dee8..642a0edabf 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2958,7 +2958,9 @@ static int push_check(int argc, const char **argv, const char *prefix UNUSED) rs->src); } } + refspec_clear(&refspec); + free_refs(local_refs); } free(head); diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh index f3fff55744..135823630a 100755 --- a/t/t5531-deep-submodule-push.sh +++ b/t/t5531-deep-submodule-push.sh @@ -8,6 +8,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=1 export GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup ' -- cgit v1.3 From b8849e236f7a32d43ab3ba087587a336d69329b0 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:07 +0200 Subject: gpg-interface: fix misdesigned signing key interfaces The interfaces to retrieve signing keys and their IDs are misdesigned as they return string constants even though they indeed allocate memory, which leads to memory leaks. Refactor the code to instead always return allocated strings and let the callers free them accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/tag.c | 3 ++- commit.c | 9 ++++++--- gpg-interface.c | 26 +++++++++++++++----------- gpg-interface.h | 4 ++-- send-pack.c | 6 ++++-- t/t5534-push-signed.sh | 1 + 6 files changed, 30 insertions(+), 19 deletions(-) (limited to 'builtin') diff --git a/builtin/tag.c b/builtin/tag.c index a1fb218512..ab3b500543 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -160,7 +160,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid, const struct git_hash_algo *compat = the_repository->compat_hash_algo; struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT; struct strbuf compat_buf = STRBUF_INIT; - const char *keyid = get_signing_key(); + char *keyid = get_signing_key(); int ret = -1; if (sign_buffer(buffer, &sig, keyid)) @@ -190,6 +190,7 @@ out: strbuf_release(&sig); strbuf_release(&compat_sig); strbuf_release(&compat_buf); + free(keyid); return ret; } diff --git a/commit.c b/commit.c index 24ab5c1b50..ec9efc189d 100644 --- a/commit.c +++ b/commit.c @@ -1150,11 +1150,14 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid) { + char *keyid_to_free = NULL; + int ret = 0; if (!keyid || !*keyid) - keyid = get_signing_key(); + keyid = keyid_to_free = get_signing_key(); if (sign_buffer(buf, sig, keyid)) - return -1; - return 0; + ret = -1; + free(keyid_to_free); + return ret; } int parse_signed_commit(const struct commit *commit, diff --git a/gpg-interface.c b/gpg-interface.c index 6587085cd1..cf6126b5aa 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -45,8 +45,8 @@ struct gpg_format { size_t signature_size); int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); - const char *(*get_default_key)(void); - const char *(*get_key_id)(void); + char *(*get_default_key)(void); + char *(*get_key_id)(void); }; static const char *openpgp_verify_args[] = { @@ -86,9 +86,9 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature, static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); -static const char *get_default_ssh_signing_key(void); +static char *get_default_ssh_signing_key(void); -static const char *get_ssh_key_id(void); +static char *get_ssh_key_id(void); static struct gpg_format gpg_format[] = { { @@ -847,7 +847,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key) } /* Returns the first public key from an ssh-agent to use for signing */ -static const char *get_default_ssh_signing_key(void) +static char *get_default_ssh_signing_key(void) { struct child_process ssh_default_key = CHILD_PROCESS_INIT; int ret = -1; @@ -899,12 +899,16 @@ static const char *get_default_ssh_signing_key(void) return default_key; } -static const char *get_ssh_key_id(void) { - return get_ssh_key_fingerprint(get_signing_key()); +static char *get_ssh_key_id(void) +{ + char *signing_key = get_signing_key(); + char *key_id = get_ssh_key_fingerprint(signing_key); + free(signing_key); + return key_id; } /* Returns a textual but unique representation of the signing key */ -const char *get_signing_key_id(void) +char *get_signing_key_id(void) { gpg_interface_lazy_init(); @@ -916,17 +920,17 @@ const char *get_signing_key_id(void) return get_signing_key(); } -const char *get_signing_key(void) +char *get_signing_key(void) { gpg_interface_lazy_init(); if (configured_signing_key) - return configured_signing_key; + return xstrdup(configured_signing_key); if (use_format->get_default_key) { return use_format->get_default_key(); } - return git_committer_info(IDENT_STRICT | IDENT_NO_DATE); + return xstrdup(git_committer_info(IDENT_STRICT | IDENT_NO_DATE)); } const char *gpg_trust_level_to_str(enum signature_trust_level level) diff --git a/gpg-interface.h b/gpg-interface.h index 7cd98161f7..e09f12e8d0 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -80,13 +80,13 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *gpg_trust_level_to_str(enum signature_trust_level level); void set_signing_key(const char *); -const char *get_signing_key(void); +char *get_signing_key(void); /* * Returns a textual unique representation of the signing key in use * Either a GPG KeyID or a SSH Key Fingerprint */ -const char *get_signing_key_id(void); +char *get_signing_key_id(void); int check_signature(struct signature_check *sigc, const char *signature, size_t slen); void print_signature_buffer(const struct signature_check *sigc, diff --git a/send-pack.c b/send-pack.c index c37f6ab3c0..31a62e6a98 100644 --- a/send-pack.c +++ b/send-pack.c @@ -348,7 +348,8 @@ static int generate_push_cert(struct strbuf *req_buf, { const struct ref *ref; struct string_list_item *item; - char *signing_key_id = xstrdup(get_signing_key_id()); + char *signing_key_id = get_signing_key_id(); + char *signing_key = get_signing_key(); const char *cp, *np; struct strbuf cert = STRBUF_INIT; int update_seen = 0; @@ -381,7 +382,7 @@ static int generate_push_cert(struct strbuf *req_buf, if (!update_seen) goto free_return; - if (sign_buffer(&cert, &cert, get_signing_key())) + if (sign_buffer(&cert, &cert, signing_key)) die(_("failed to sign the push certificate")); packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string); @@ -394,6 +395,7 @@ static int generate_push_cert(struct strbuf *req_buf, free_return: free(signing_key_id); + free(signing_key); strbuf_release(&cert); return update_seen; } diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh index c91a62b77a..d43aee0c32 100755 --- a/t/t5534-push-signed.sh +++ b/t/t5534-push-signed.sh @@ -5,6 +5,7 @@ test_description='signed push' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-gpg.sh -- cgit v1.3 From 860b6780163ade3bb705d6565619ec13efcc77c6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:23 +0200 Subject: builtin/repack: fix leaking line buffer when packing promisors In `repack_promisor_objects()` we read output from git-pack-objects(1) line by line, using `strbuf_getline_lf()`. We never free the line buffer, causing a memory leak. Plug it. This leak is being hit in t5616, but plugging it alone is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/repack.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'builtin') diff --git a/builtin/repack.c b/builtin/repack.c index 62cfa50c50..2b9bf0318a 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -425,9 +425,11 @@ static void repack_promisor_objects(const struct pack_objects_args *args, free(promisor_name); } + fclose(out); if (finish_command(&cmd)) die(_("could not finish pack-objects to repack promisor objects")); + strbuf_release(&line); } struct pack_geometry { -- cgit v1.3 From 149c83e0aa1ee75b2da9c27ce5a819025260b4da Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:26 +0200 Subject: builtin/pack-objects: plug leaking list of keep-packs The `--keep-pack` option of git-pack-objects(1) populates the arguments into a string list. And while the list is marked as `NODUP` and thus won't duplicate the strings, the list entries themselves still need to be free'd. We don't though, causing a leak. Plug it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 1 + t/t5616-partial-clone.sh | 1 + 2 files changed, 2 insertions(+) (limited to 'builtin') diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c481feadbf..ab78d72e27 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -4641,6 +4641,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) cleanup: clear_packing_data(&to_pack); list_objects_filter_release(&filter_options); + string_list_clear(&keep_pack_list, 0); strvec_clear(&rp); return 0; diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 2da7291e37..467c46dccf 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -5,6 +5,7 @@ test_description='git partial clone' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # create a normal "src" repo where we can later create new commits. -- cgit v1.3 From ee087c29c89ad54980f2521641f5f182f6adbc79 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:31 +0200 Subject: builtin/grep: fix leaking object context Even when `get_oid_with_context()` fails it may have allocated some data in the object context. But we do not release it in git-grep(1) when the call fails, leading to a memory leak. Plug it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/grep.c | 1 + t/t6132-pathspec-exclude.sh | 1 + t/t6135-pathspec-with-attrs.sh | 2 ++ 3 files changed, 4 insertions(+) (limited to 'builtin') diff --git a/builtin/grep.c b/builtin/grep.c index dfc3c3e8bd..dda4582d64 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1133,6 +1133,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) &oid, &oc)) { if (seen_dashdash) die(_("unable to resolve revision: %s"), arg); + object_context_release(&oc); break; } diff --git a/t/t6132-pathspec-exclude.sh b/t/t6132-pathspec-exclude.sh index 9fdafeb1e9..f31c09c056 100755 --- a/t/t6132-pathspec-exclude.sh +++ b/t/t6132-pathspec-exclude.sh @@ -2,6 +2,7 @@ test_description='test case exclude pathspec' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t6135-pathspec-with-attrs.sh b/t/t6135-pathspec-with-attrs.sh index 120dcd74a5..794bc7daf0 100755 --- a/t/t6135-pathspec-with-attrs.sh +++ b/t/t6135-pathspec-with-attrs.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='test labels in pathspecs' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup a tree' ' -- cgit v1.3 From 68bd0a94bedf520bc67ca38216e839770f291296 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:34 +0200 Subject: builtin/fmt-merge-msg: fix leaking buffers Fix leaking input and output buffers in git-fmt-merge-msg(1). Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fmt-merge-msg.c | 2 ++ t/t6200-fmt-merge-msg.sh | 1 + 2 files changed, 3 insertions(+) (limited to 'builtin') diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 957786d1b3..0b162f8fab 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -67,6 +67,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix) return ret; write_in_full(STDOUT_FILENO, output.buf, output.len); + strbuf_release(&input); + strbuf_release(&output); free(inpath); return 0; } diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh index 5a221f8ef1..ac57b0e4ae 100755 --- a/t/t6200-fmt-merge-msg.sh +++ b/t/t6200-fmt-merge-msg.sh @@ -8,6 +8,7 @@ test_description='fmt-merge-msg test' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY/lib-gpg.sh" -- cgit v1.3 From 46f6ca2a68e02dd68132ed0b64cd55a8b6569e29 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 5 Sep 2024 12:09:41 +0200 Subject: builtin/repack: fix leaking keep-pack list The list of packs to keep is populated via a command line option but never free'd. Plug this memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/repack.c | 1 + t/t6500-gc.sh | 1 + t/t7703-repack-geometric.sh | 1 + 3 files changed, 3 insertions(+) (limited to 'builtin') diff --git a/builtin/repack.c b/builtin/repack.c index 2b9bf0318a..367e970dcf 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -1525,6 +1525,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) } cleanup: + string_list_clear(&keep_pack_list, 0); string_list_clear(&names, 1); existing_packs_release(&existing); free_pack_geometry(&geometry); diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 1b5909d1b7..58654b3437 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -3,6 +3,7 @@ test_description='basic git gc tests ' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh index 9fc1626fbf..8877aea98b 100755 --- a/t/t7703-repack-geometric.sh +++ b/t/t7703-repack-geometric.sh @@ -2,6 +2,7 @@ test_description='git repack --geometric works correctly' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh GIT_TEST_MULTI_PACK_INDEX=0 -- cgit v1.3