git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Michael J Gruber <git@drmicha.warpmail.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 3/7] verify_signed_buffer: use tempfile object
Date: Thu, 16 Jun 2016 05:35:01 -0400	[thread overview]
Message-ID: <20160616093501.GC15851@sigill.intra.peff.net> (raw)
In-Reply-To: <20160616093248.GA15130@sigill.intra.peff.net>

We use git_mkstemp to create a temporary file, and try to
clean it up in all exit paths from the function. But that
misses any cases where we die by signal, or by calling die()
in a sub-function. In addition, we missed one of the exit
paths.

Let's convert to using a tempfile object, which handles the
hard cases for us, and add the missing cleanup call. Note
that we would not simply want to rely on program exit to
catch our missed cleanup, as this function may be called
many times in a single program (for the same reason, we use
a static tempfile instead of heap-allocating a new one; that
gives an upper bound on our memory usage).

Signed-off-by: Jeff King <peff@peff.net>
---
 gpg-interface.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 216cad8..854c1e5 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -3,6 +3,7 @@
 #include "strbuf.h"
 #include "gpg-interface.h"
 #include "sigchain.h"
+#include "tempfile.h"
 
 static char *configured_signing_key;
 static const char *gpg_program = "gpg";
@@ -208,28 +209,32 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 			 struct strbuf *gpg_output, struct strbuf *gpg_status)
 {
 	struct child_process gpg = CHILD_PROCESS_INIT;
-	char path[PATH_MAX];
+	static struct tempfile temp;
 	int fd, ret;
 	struct strbuf buf = STRBUF_INIT;
 
-	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
+	fd = mks_tempfile_t(&temp, ".git_vtag_tmpXXXXXX");
 	if (fd < 0)
-		return error_errno(_("could not create temporary file '%s'"), path);
-	if (write_in_full(fd, signature, signature_size) < 0)
-		return error_errno(_("failed writing detached signature to '%s'"), path);
+		return error_errno(_("could not create temporary file"));
+	if (write_in_full(fd, signature, signature_size) < 0) {
+		error_errno(_("failed writing detached signature to '%s'"),
+			    temp.filename.buf);
+		delete_tempfile(&temp);
+		return -1;
+	}
 	close(fd);
 
 	argv_array_pushl(&gpg.args,
 			 gpg_program,
 			 "--status-fd=1",
-			 "--verify", path, "-",
+			 "--verify", temp.filename.buf, "-",
 			 NULL);
 	gpg.in = -1;
 	gpg.out = -1;
 	if (gpg_output)
 		gpg.err = -1;
 	if (start_command(&gpg)) {
-		unlink(path);
+		delete_tempfile(&temp);
 		return error(_("could not run gpg."));
 	}
 
@@ -249,7 +254,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 	ret = finish_command(&gpg);
 	sigchain_pop(SIGPIPE);
 
-	unlink_or_warn(path);
+	delete_tempfile(&temp);
 
 	ret |= !strstr(gpg_status->buf, "\n[GNUPG:] GOODSIG ");
 	strbuf_release(&buf); /* no matter it was used or not */
-- 
2.9.0.160.g4984cba


  parent reply	other threads:[~2016-06-16  9:35 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-16  9:32 [PATCH 0/7] gpg-interface cleanups Jeff King
2016-06-16  9:33 ` [PATCH 1/7] gpg-interface: use child_process.args Jeff King
2016-06-16  9:34 ` [PATCH 2/7] verify_signed_buffer: drop pbuf variable Jeff King
2016-06-16  9:35 ` Jeff King [this message]
2016-06-16  9:37 ` [PATCH 4/7] run-command: add pipe_command helper Jeff King
2016-06-17 20:03   ` Eric Sunshine
2016-06-17 23:28     ` Jeff King
2016-06-17 23:31       ` Eric Sunshine
2016-06-16  9:38 ` [PATCH 5/7] verify_signed_buffer: use pipe_command Jeff King
2016-06-16  9:39 ` [PATCH 6/7] sign_buffer: " Jeff King
2016-06-16  9:42 ` [PATCH 7/7] gpg-interface: check gpg signature creation status Jeff King
2016-06-16 18:20 ` [PATCH 0/7] gpg-interface cleanups Junio C Hamano
2016-06-17  9:20   ` Michael J Gruber
2016-06-17  9:21     ` Jeff King
2016-06-17 23:38 ` [PATCH v2 " Jeff King
2016-06-17 23:38   ` [PATCH v2 1/7] gpg-interface: use child_process.args Jeff King
2016-06-17 23:38   ` [PATCH v2 2/7] verify_signed_buffer: drop pbuf variable Jeff King
2016-06-17 23:38   ` [PATCH v2 3/7] verify_signed_buffer: use tempfile object Jeff King
2016-06-17 23:38   ` [PATCH v2 4/7] run-command: add pipe_command helper Jeff King
2016-06-17 23:38   ` [PATCH v2 5/7] verify_signed_buffer: use pipe_command Jeff King
2016-06-17 23:38   ` [PATCH v2 6/7] sign_buffer: " Jeff King
2016-06-17 23:38   ` [PATCH v2 7/7] gpg-interface: check gpg signature creation status Jeff King

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=20160616093501.GC15851@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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).