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

Re: [PATCH v3 1/4] builtin/verify-tag.c: Ignore SIGPIPE on gpg-interface

From
Santiago Torres <santiago@nyu.edu>
Date
Apr 3, 2016, 21:46 UTC
Message-ID
<20160403214606.GB28933@LykOS>
In-Reply-To
<5700BD33.6090602@kdbg.org>
On Sun, Apr 03, 2016 at 08:50:27AM +0200, Johannes Sixt wrote:
Show 55 quoted lines
> Am 03.04.2016 um 01:16 schrieb santiago@nyu.edu:
> > From: Santiago Torres <santiago@nyu.edu>
> > 
> > The verify_signed_buffer comand might cause a SIGPIPE signal when the
> > gpg child process terminates early (due to a bad keyid, for example) and
> > git tries to write to it afterwards. Previously, ignoring SIGPIPE was
> > done on the builtin/gpg-verify.c command to avoid this issue. However,
> > any other caller who wanted to use the verify_signed_buffer command
> > would have to include this signal call.
> > 
> > Instead, we use sigchain_push(SIGPIPE, SIG_IGN) on the
> > verify_signed_buffer call (pretty much like in sign_buffer()) so
> > that any caller is not required to perform this task. This will avoid
> > possible mistakes by further developers using verify_signed_buffer.
> > 
> > Signed-off-by: Santiago Torres <santiago@nyu.edu>
> > ---
> > Notes:
> >   I dropped the multiline comment altogheter.
> > 
> >   builtin/verify-tag.c | 3 ---
> >   gpg-interface.c      | 3 +++
> >   2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
> > index 00663f6..77f070a 100644
> > --- a/builtin/verify-tag.c
> > +++ b/builtin/verify-tag.c
> > @@ -95,9 +95,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
> >   	if (verbose)
> >   		flags |= GPG_VERIFY_VERBOSE;
> > 
> > -	/* sometimes the program was terminated because this signal
> > -	 * was received in the process of writing the gpg input: */
> > -	signal(SIGPIPE, SIG_IGN);
> >   	while (i < argc)
> >   		if (verify_tag(argv[i++], flags))
> >   			had_error = 1;
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index 3dc2fe3..c1f6b2d 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -232,6 +232,8 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
> >   	if (gpg_output)
> >   		gpg.err = -1;
> >   	args_gpg[3] = path;
> > +
> > +	sigchain_push(SIGPIPE, SIG_IGN);
> >   	if (start_command(&gpg)) {
> >   		unlink(path);
> >   		return error(_("could not run gpg."));
> 
> But no sigchain_pop() in the error path that we see here?
> 
> Perhaps you can even defer the sigchain_push() until after start_command()?

Yes, after reviewing where the sigchain_push() call is made on sign_buffer(), I should put the push call after starting the command in the same way sign_buffer() does.

Thanks! -Santiago.

Previous: Johannes SixtNext: santiago@nyu.edu
Message 5 of 24 in “tag: move PGP verification code to tag.c”
  1. 0/4 tag: move PGP verification code to tag.csantiago@nyu.edu, Apr 2, 2016
  2. 1/4 builtin/verify-tag.c: Ignore SIGPIPE on gpg-interfacesantiago@nyu.edu, Apr 2, 2016
  3. Jeff KingApr 3, 2016
  4. Johannes SixtApr 3, 2016
  5. Santiago TorresApr 3, 2016
  6. 2/4 t/t7030-verify-tag.sh: Adds validation for multiple tagssantiago@nyu.edu, Apr 2, 2016
  7. Jeff KingApr 3, 2016
  8. Eric SunshineApr 3, 2016
  9. Jeff KingApr 3, 2016
  10. Santiago TorresApr 3, 2016
  11. Eric SunshineApr 4, 2016
  12. Jeff KingApr 4, 2016
  13. 3/4 builtin/verify-tag: move verification code to tag.csantiago@nyu.edu, Apr 2, 2016
  14. Jeff KingApr 3, 2016
  15. Eric SunshineApr 3, 2016
  16. Eric SunshineApr 3, 2016
  17. Santiago TorresApr 3, 2016
  18. 4/4 tag: use pgp_verify_function in tag -v callsantiago@nyu.edu, Apr 2, 2016
  19. Jeff KingApr 3, 2016
  20. Santiago TorresApr 3, 2016
  21. Santiago TorresApr 4, 2016
  22. Jeff KingApr 4, 2016
  23. Santiago TorresApr 4, 2016
  24. Jeff KingApr 4, 2016

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.