git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Calvin Wan <calvinwan@google.com>,
	Josh Steadmon <steadmon@google.com>,
	Junio C Hamano <gitster@pobox.com>,
	Elijah Newren <newren@gmail.com>, Toon claes <toon@iotcl.com>
Subject: [PATCH v2 10/22] send-pack: fix leaking push cert nonce
Date: Thu, 5 Sep 2024 12:09:04 +0200	[thread overview]
Message-ID: <a1baba39bc555fc8b9d82b0148c7ad5449ed3bf9.1725530720.git.ps@pks.im> (raw)
In-Reply-To: <cover.1725530720.git.ps@pks.im>

When retrieving the push cert nonce from the server, we first store the
constant returned by `server_feature_value()` and then, if the nonce is
valid, we duplicate the nonce memory to a NUL-terminated string, so that
we can pass it to `generate_push_cert()`. We never free the latter and
thus cause a memory leak.

Fix this by storing the limited-lifetime nonce into a scope-local
variable such that the long-lived, allocated nonce can be easily freed
without having to cast away its constness.

This leak was exposed by t5534, but fixing it is not sufficient to make
the whole test suite leak free.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 send-pack.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index b224ef9fc5e..c37f6ab3c07 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -501,7 +501,7 @@ int send_pack(struct send_pack_args *args,
 	unsigned cmds_sent = 0;
 	int ret;
 	struct async demux;
-	const char *push_cert_nonce = NULL;
+	char *push_cert_nonce = NULL;
 	struct packet_reader reader;
 	int use_bitmaps;
 
@@ -550,10 +550,11 @@ int send_pack(struct send_pack_args *args,
 
 	if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
 		size_t len;
-		push_cert_nonce = server_feature_value("push-cert", &len);
-		if (push_cert_nonce) {
-			reject_invalid_nonce(push_cert_nonce, len);
-			push_cert_nonce = xmemdupz(push_cert_nonce, len);
+		const char *nonce = server_feature_value("push-cert", &len);
+
+		if (nonce) {
+			reject_invalid_nonce(nonce, len);
+			push_cert_nonce = xmemdupz(nonce, len);
 		} else if (args->push_cert == SEND_PACK_PUSH_CERT_ALWAYS) {
 			die(_("the receiving end does not support --signed push"));
 		} else if (args->push_cert == SEND_PACK_PUSH_CERT_IF_ASKED) {
@@ -771,5 +772,6 @@ int send_pack(struct send_pack_args *args,
 	oid_array_clear(&commons);
 	strbuf_release(&req_buf);
 	strbuf_release(&cap_buf);
+	free(push_cert_nonce);
 	return ret;
 }
-- 
2.46.0.519.g2e7b89e038.dirty


  parent reply	other threads:[~2024-09-05 10:09 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-26  7:21 [PATCH 00/22] Memory leak fixes (pt.6) Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 01/22] t/test-lib: allow skipping leak checks for passing tests Patrick Steinhardt
2024-08-27 22:38   ` Junio C Hamano
2024-08-29 14:15   ` Toon claes
2024-08-30  9:00     ` Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 02/22] fetch-pack: fix memory leaks on fetch negotiation Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 03/22] send-pack: fix leaking common object IDs Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 04/22] builtin/push: fix leaking refspec query result Patrick Steinhardt
2024-08-30 21:59   ` Junio C Hamano
2024-09-02  9:27     ` Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 05/22] upload-pack: fix leaking child process data on reachability checks Patrick Steinhardt
2024-08-30 22:30   ` Junio C Hamano
2024-08-26  7:21 ` [PATCH 06/22] submodule: fix leaking fetch task data Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 07/22] builtin/submodule--helper: fix leaking refs on push-check Patrick Steinhardt
2024-08-26  7:21 ` [PATCH 08/22] remote: fix leaking tracking refs Patrick Steinhardt
2024-09-04 21:50   ` Junio C Hamano
2024-08-26  7:21 ` [PATCH 09/22] remote: fix leak in reachability check of a remote-tracking ref Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 10/22] send-pack: fix leaking push cert nonce Patrick Steinhardt
2024-09-04 22:08   ` Junio C Hamano
2024-08-26  7:22 ` [PATCH 11/22] gpg-interface: fix misdesigned signing key interfaces Patrick Steinhardt
2024-09-04 22:09   ` Junio C Hamano
2024-08-26  7:22 ` [PATCH 12/22] object: clear grafts when clearing parsed object pool Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 13/22] shallow: free grafts when unregistering them Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 14/22] shallow: fix leaking members of `struct shallow_info` Patrick Steinhardt
2024-08-29 14:16   ` Toon claes
2024-08-29 16:07     ` Junio C Hamano
2024-08-30  9:00       ` Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 15/22] negotiator/skipping: fix leaking commit entries Patrick Steinhardt
2024-08-28 20:29   ` Calvin Wan
2024-08-28 22:19     ` Josh Steadmon
2024-08-29  8:41       ` Patrick Steinhardt
2024-08-29 17:29         ` Calvin Wan
2024-08-26  7:22 ` [PATCH 16/22] builtin/repack: fix leaking line buffer when packing promisors Patrick Steinhardt
2024-09-04 22:27   ` Junio C Hamano
2024-08-26  7:22 ` [PATCH 17/22] builtin/pack-objects: plug leaking list of keep-packs Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 18/22] builtin/grep: fix leaking object context Patrick Steinhardt
2024-09-04 22:36   ` Junio C Hamano
2024-08-26  7:22 ` [PATCH 19/22] builtin/fmt-merge-msg: fix leaking buffers Patrick Steinhardt
2024-08-26  7:22 ` [PATCH 20/22] match-trees: fix leaking prefixes in `shift_tree()` Patrick Steinhardt
2024-09-04 22:42   ` Junio C Hamano
2024-08-26  7:22 ` [PATCH 21/22] merge-ort: fix two leaks when handling directory rename modifications Patrick Steinhardt
2024-09-04 22:56   ` Junio C Hamano
2024-09-05  2:01     ` Elijah Newren
2024-08-26  7:22 ` [PATCH 22/22] builtin/repack: fix leaking keep-pack list Patrick Steinhardt
2024-09-04 23:01 ` [PATCH 00/22] Memory leak fixes (pt.6) Junio C Hamano
2024-09-05 10:08 ` [PATCH v2 " Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 01/22] t/test-lib: allow skipping leak checks for passing tests Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 02/22] fetch-pack: fix memory leaks on fetch negotiation Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 03/22] send-pack: fix leaking common object IDs Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 04/22] builtin/push: fix leaking refspec query result Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 05/22] upload-pack: fix leaking child process data on reachability checks Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 06/22] submodule: fix leaking fetch task data Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 07/22] builtin/submodule--helper: fix leaking refs on push-check Patrick Steinhardt
2024-09-05 10:08   ` [PATCH v2 08/22] remote: fix leaking tracking refs Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 09/22] remote: fix leak in reachability check of a remote-tracking ref Patrick Steinhardt
2024-09-05 10:09   ` Patrick Steinhardt [this message]
2024-09-05 10:09   ` [PATCH v2 11/22] gpg-interface: fix misdesigned signing key interfaces Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 12/22] object: clear grafts when clearing parsed object pool Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 13/22] shallow: free grafts when unregistering them Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 14/22] shallow: fix leaking members of `struct shallow_info` Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 15/22] negotiator/skipping: fix leaking commit entries Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 16/22] builtin/repack: fix leaking line buffer when packing promisors Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 17/22] builtin/pack-objects: plug leaking list of keep-packs Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 18/22] builtin/grep: fix leaking object context Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 19/22] builtin/fmt-merge-msg: fix leaking buffers Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 20/22] match-trees: fix leaking prefixes in `shift_tree()` Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 21/22] merge-ort: fix two leaks when handling directory rename modifications Patrick Steinhardt
2024-09-05 10:09   ` [PATCH v2 22/22] builtin/repack: fix leaking keep-pack list Patrick Steinhardt
2024-09-08 21:39   ` [PATCH v2 00/22] Memory leak fixes (pt.6) Junio C Hamano
2024-09-12 20:29   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a1baba39bc555fc8b9d82b0148c7ad5449ed3bf9.1725530720.git.ps@pks.im \
    --to=ps@pks.im \
    --cc=calvinwan@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=steadmon@google.com \
    --cc=toon@iotcl.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).