[PATCH 2/2] git: ensure 0/1/2 are open in main()

Thomas Rast· Jul 16, 2013, 09:27 UTC · lore

[PATCH 1/2] daemon/shell: refactor redirection of 0/1/2 from /dev/null

Both daemon.c and shell.c contain logic to open FDs 0/1/2 from /dev/null if they are not already open. Move the function in daemon.c to setup.c and use it in shell.c, too.

While there, remove a 'not' that inverted the meaning of the comment. The point is indeed to *avoid* messing up.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 cache.h  |  2 ++
 daemon.c | 12 ------------
 setup.c  | 12 ++++++++++++
 shell.c  | 12 +++---------
 4 files changed, 17 insertions(+), 21 deletions(-)

Show changes to 4 files +17 −21

cache.h, daemon.c, setup.c, shell.c

diff --git a/cache.h b/cache.h
index dd0fb33..f007724 100644
--- a/cache.h
+++ b/cache.h
@@ -425,6 +425,8 @@ extern void verify_filename(const char *prefix,
 extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);
 extern int init_db(const char *template_dir, unsigned int flags);
 
+extern void sanitize_stdfds(void);
+
 #define alloc_nr(x) (((x)+16)*3/2)
 
 /*
diff --git a/daemon.c b/daemon.c
index 6aeddcb..973ec38 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1047,18 +1047,6 @@ static int service_loop(struct socketlist *socklist)
 	}
 }
 
-/* if any standard file descriptor is missing open it to /dev/null */
-static void sanitize_stdfds(void)
-{
-	int fd = open("/dev/null", O_RDWR, 0);
-	while (fd != -1 && fd < 2)
-		fd = dup(fd);
-	if (fd == -1)
-		die_errno("open /dev/null or dup failed");
-	if (fd > 2)
-		close(fd);
-}
-
 #ifdef NO_POSIX_GOODIES
 
 struct credentials;
diff --git a/setup.c b/setup.c
index 94c1e61..88aab94 100644
--- a/setup.c
+++ b/setup.c
@@ -908,3 +908,15 @@ const char *resolve_gitdir(const char *suspect)
 		return suspect;
 	return read_gitfile(suspect);
 }
+
+/* if any standard file descriptor is missing open it to /dev/null */
+void sanitize_stdfds(void)
+{
+	int fd = open("/dev/null", O_RDWR, 0);
+	while (fd != -1 && fd < 2)
+		fd = dup(fd);
+	if (fd == -1)
+		die_errno("open /dev/null or dup failed");
+	if (fd > 2)
+		close(fd);
+}
diff --git a/shell.c b/shell.c
index 1429870..66350b2 100644
--- a/shell.c
+++ b/shell.c
@@ -147,7 +147,6 @@ int main(int argc, char **argv)
 	char *prog;
 	const char **user_argv;
 	struct commands *cmd;
-	int devnull_fd;
 	int count;
 
 	git_setup_gettext();
@@ -156,15 +155,10 @@ int main(int argc, char **argv)
 
 	/*
 	 * Always open file descriptors 0/1/2 to avoid clobbering files
-	 * in die().  It also avoids not messing up when the pipes are
-	 * dup'ed onto stdin/stdout/stderr in the child processes we spawn.
+	 * in die().  It also avoids messing up when the pipes are dup'ed
+	 * onto stdin/stdout/stderr in the child processes we spawn.
 	 */
-	devnull_fd = open("/dev/null", O_RDWR);
-	while (devnull_fd >= 0 && devnull_fd <= 2)
-		devnull_fd = dup(devnull_fd);
-	if (devnull_fd == -1)
-		die_errno("opening /dev/null failed");
-	close (devnull_fd);
+	sanitize_stdfds();
 
 	/*
 	 * Special hack to pretend to be a CVS server

-- 
1.8.3.2.998.g1d087bc

Thomas Rast· Jul 16, 2013, 09:27 UTC · re: Thomas Rast · lore

Not having an open FD in the 0--2 range can lead to strange results, for example, a subsequent open() may return 2 (stderr) and then a die() would clobber this file.

git-daemon and git-shell already guarded against this, but apparently users also manage to trip over it in other git commands. So we call sanitize_stdfds() during main git startup.

Since these FDs are inherited, this covers all use of 'git foo ...', and all internal C commands when called directly. It does not fix shell/perl commands called directly.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 git.c | 7 +++++++
 1 file changed, 7 insertions(+)

Show changes to git.c +7 −0

diff --git a/git.c b/git.c
index 4359086..6104d5e 100644
--- a/git.c
+++ b/git.c
@@ -525,6 +525,13 @@ int main(int argc, char **av)
 	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();
+
 	git_setup_gettext();
 
 	/*

-- 
1.8.3.2.998.g1d087bc

patch, 2 partsgit: ensure 0/1/2 are open in main()