git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "dmh@ucar.edu" <dmh@ucar.edu>
Cc: git@vger.kernel.org
Subject: [PATCH 3/5] common-main: call sanitize_stdfds()
Date: Fri, 1 Jul 2016 02:06:02 -0400	[thread overview]
Message-ID: <20160701060602.GC4593@sigill.intra.peff.net> (raw)
In-Reply-To: <20160701055532.GA4488@sigill.intra.peff.net>

This is setup that should be done in every program for
safety, but we never got around to adding it everywhere (so
builtins benefited from the call in git.c, but any external
commands did not). Putting it in the common main() gives us
this safety everywhere.

Note that the case in daemon.c is a little funny. We wait
until we know whether we want to daemonize, and then either:

 - call daemonize(), which will close stdio and reopen it to
   /dev/null under the hood

 - sanitize_stdfds(), to fix up any odd cases

But that is way too late; the point of sanitizing is to give
us reliable descriptors on 0/1/2, and we will already have
executed code, possibly called die(), etc. The sanitizing
should be the very first thing that happens.

With this patch, git-daemon will sanitize first, and can
remove the call in the non-daemonize case. It does mean that
daemonize() may just end up closing the descriptors we
opened, but that's not a big deal (it's not wrong to do so,
nor is it really less optimal than the case where our parent
process redirected us from /dev/null ahead of time).

Signed-off-by: Jeff King <peff@peff.net>
---
 common-main.c | 9 ++++++++-
 daemon.c      | 3 +--
 git.c         | 7 -------
 shell.c       | 7 -------
 4 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/common-main.c b/common-main.c
index 57c912a..353c6ea 100644
--- a/common-main.c
+++ b/common-main.c
@@ -1,4 +1,4 @@
-#include "git-compat-util.h"
+#include "cache.h"
 #include "exec_cmd.h"
 
 int main(int argc, char **av)
@@ -9,6 +9,13 @@ int main(int argc, char **av)
 	 */
 	const char **argv = (const char **)av;
 
+	/*
+	 * Always open file descriptors 0/1/2 to avoid clobbering files
+	 * in die().  It also avoids messing up when the pipes are dup'ed
+	 * onto stdin/stdout/stderr in the child processes we spawn.
+	 */
+	sanitize_stdfds();
+
 	argv[0] = git_extract_argv0_path(argv[0]);
 
 	return cmd_main(argc, argv);
diff --git a/daemon.c b/daemon.c
index d33ee5a..8646f33 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1378,8 +1378,7 @@ int cmd_main(int argc, const char **argv)
 	if (detach) {
 		if (daemonize())
 			die("--detach not supported on this platform");
-	} else
-		sanitize_stdfds();
+	}
 
 	if (pid_file)
 		write_file(pid_file, "%"PRIuMAX, (uintmax_t) getpid());
diff --git a/git.c b/git.c
index 3b4e12d..b65083c 100644
--- a/git.c
+++ b/git.c
@@ -639,13 +639,6 @@ int cmd_main(int argc, const char **argv)
 	if (!cmd)
 		cmd = "git-help";
 
-	/*
-	 * Always open file descriptors 0/1/2 to avoid clobbering files
-	 * in die().  It also avoids messing up when the pipes are dup'ed
-	 * onto stdin/stdout/stderr in the child processes we spawn.
-	 */
-	sanitize_stdfds();
-
 	restore_sigpipe_to_default();
 
 	git_setup_gettext();
diff --git a/shell.c b/shell.c
index ca00807..5e70acb 100644
--- a/shell.c
+++ b/shell.c
@@ -147,13 +147,6 @@ int cmd_main(int argc, const char **argv)
 
 	git_setup_gettext();
 
-	/*
-	 * Always open file descriptors 0/1/2 to avoid clobbering files
-	 * in die().  It also avoids messing up when the pipes are dup'ed
-	 * onto stdin/stdout/stderr in the child processes we spawn.
-	 */
-	sanitize_stdfds();
-
 	/*
 	 * Special hack to pretend to be a CVS server
 	 */
-- 
2.9.0.317.g65b4e7c


  parent reply	other threads:[~2016-07-01  6:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30 21:24 git-credentials-store.exe crash dmh
2016-07-01  4:07 ` Jeff King
2016-07-01  5:55   ` [PATCH 0/5] consistent setup code for external commands Jeff King
2016-07-01  5:58     ` [PATCH 1/5] add an extra level of indirection to main() Jeff King
2016-07-01  8:04       ` Johannes Schindelin
2016-07-01  8:19         ` Jeff King
2016-07-01 13:39           ` Johannes Schindelin
2016-07-01 22:38             ` Jeff King
2016-07-02  6:52               ` Johannes Schindelin
2016-07-01  6:04     ` [PATCH 2/5] common-main: call git_extract_argv0_path() Jeff King
2016-07-01  8:05       ` Johannes Schindelin
2016-07-01  6:06     ` Jeff King [this message]
2016-07-01  6:06     ` [PATCH 4/5] common-main: call restore_sigpipe_to_default() Jeff King
2016-07-01  6:07     ` [PATCH 5/5] common-main: call git_setup_gettext() Jeff King
2016-07-01  7:45     ` [PATCH 0/5] consistent setup code for external commands Johannes Schindelin
2016-07-01 22:19     ` Junio C Hamano
2016-07-01 22:39       ` Jeff King
2016-07-06 15:17         ` Junio C Hamano
2016-07-06 15:36           ` Johannes Schindelin
2016-07-01  7:38   ` git-credentials-store.exe crash Johannes Schindelin
2016-07-01  7:43     ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20160701060602.GC4593@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dmh@ucar.edu \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).