{"thread":{"id":"34449","subject":"[PATCH 2/2] git: ensure 0/1/2 are open in main()","startedAt":"2013-07-16T09:27:36Z","lastAt":"2013-07-16T09:27:37Z","messageCount":2,"participants":["Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"223516","messageId":"1fe4ff3df32aa1836ed3672e749f582047745bc9.1373966758.git.trast@inf.ethz.ch","threadId":"34449","inReplyTo":"7vtxjzlmaf.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] daemon/shell: refactor redirection of 0/1/2 from /dev/null","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-16T09:27:36Z","receivedAt":"2013-07-16T09:27:36Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Both daemon.c and shell.c contain logic to open FDs 0/1/2 from\n/dev/null if they are not already open.  Move the function in daemon.c\nto setup.c and use it in shell.c, too.\n\nWhile there, remove a 'not' that inverted the meaning of the comment.\nThe point is indeed to *avoid* messing up.\n\nSigned-off-by: Thomas Rast <trast@inf.ethz.ch>\n---\n cache.h  |  2 ++\n daemon.c | 12 ------------\n setup.c  | 12 ++++++++++++\n shell.c  | 12 +++---------\n 4 files changed, 17 insertions(+), 21 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex dd0fb33..f007724 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -425,6 +425,8 @@ extern void verify_filename(const char *prefix,\n extern int set_git_dir_init(const char *git_dir, const char *real_git_dir, int);\n extern int init_db(const char *template_dir, unsigned int flags);\n \n+extern void sanitize_stdfds(void);\n+\n #define alloc_nr(x) (((x)+16)*3/2)\n \n /*\ndiff --git a/daemon.c b/daemon.c\nindex 6aeddcb..973ec38 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -1047,18 +1047,6 @@ static int service_loop(struct socketlist *socklist)\n \t}\n }\n \n-/* if any standard file descriptor is missing open it to /dev/null */\n-static void sanitize_stdfds(void)\n-{\n-\tint fd = open(\"/dev/null\", O_RDWR, 0);\n-\twhile (fd != -1 && fd < 2)\n-\t\tfd = dup(fd);\n-\tif (fd == -1)\n-\t\tdie_errno(\"open /dev/null or dup failed\");\n-\tif (fd > 2)\n-\t\tclose(fd);\n-}\n-\n #ifdef NO_POSIX_GOODIES\n \n struct credentials;\ndiff --git a/setup.c b/setup.c\nindex 94c1e61..88aab94 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -908,3 +908,15 @@ const char *resolve_gitdir(const char *suspect)\n \t\treturn suspect;\n \treturn read_gitfile(suspect);\n }\n+\n+/* if any standard file descriptor is missing open it to /dev/null */\n+void sanitize_stdfds(void)\n+{\n+\tint fd = open(\"/dev/null\", O_RDWR, 0);\n+\twhile (fd != -1 && fd < 2)\n+\t\tfd = dup(fd);\n+\tif (fd == -1)\n+\t\tdie_errno(\"open /dev/null or dup failed\");\n+\tif (fd > 2)\n+\t\tclose(fd);\n+}\ndiff --git a/shell.c b/shell.c\nindex 1429870..66350b2 100644\n--- a/shell.c\n+++ b/shell.c\n@@ -147,7 +147,6 @@ int main(int argc, char **argv)\n \tchar *prog;\n \tconst char **user_argv;\n \tstruct commands *cmd;\n-\tint devnull_fd;\n \tint count;\n \n \tgit_setup_gettext();\n@@ -156,15 +155,10 @@ int main(int argc, char **argv)\n \n \t/*\n \t * Always open file descriptors 0/1/2 to avoid clobbering files\n-\t * in die().  It also avoids not messing up when the pipes are\n-\t * dup'ed onto stdin/stdout/stderr in the child processes we spawn.\n+\t * in die().  It also avoids messing up when the pipes are dup'ed\n+\t * onto stdin/stdout/stderr in the child processes we spawn.\n \t */\n-\tdevnull_fd = open(\"/dev/null\", O_RDWR);\n-\twhile (devnull_fd >= 0 && devnull_fd <= 2)\n-\t\tdevnull_fd = dup(devnull_fd);\n-\tif (devnull_fd == -1)\n-\t\tdie_errno(\"opening /dev/null failed\");\n-\tclose (devnull_fd);\n+\tsanitize_stdfds();\n \n \t/*\n \t * Special hack to pretend to be a CVS server\n-- \n1.8.3.2.998.g1d087bc\n"},{"id":"223515","messageId":"65e2cdb28e76de8f669f3d58556fa9b8da944c5b.1373966758.git.trast@inf.ethz.ch","threadId":"34449","inReplyTo":"1fe4ff3df32aa1836ed3672e749f582047745bc9.1373966758.git.trast@inf.ethz.ch","subject":"[PATCH 2/2] git: ensure 0/1/2 are open in main()","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-07-16T09:27:37Z","receivedAt":"2013-07-16T09:27:37Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Not having an open FD in the 0--2 range can lead to strange results,\nfor example, a subsequent open() may return 2 (stderr) and then a\ndie() would clobber this file.\n\ngit-daemon and git-shell already guarded against this, but apparently\nusers also manage to trip over it in other git commands.  So we call\nsanitize_stdfds() during main git startup.\n\nSince these FDs are inherited, this covers all use of 'git foo ...',\nand all internal C commands when called directly.  It does not fix\nshell/perl commands called directly.\n\nSigned-off-by: Thomas Rast <trast@inf.ethz.ch>\n---\n git.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/git.c b/git.c\nindex 4359086..6104d5e 100644\n--- a/git.c\n+++ b/git.c\n@@ -525,6 +525,13 @@ int main(int argc, char **av)\n \tif (!cmd)\n \t\tcmd = \"git-help\";\n \n+\t/*\n+\t * Always open file descriptors 0/1/2 to avoid clobbering files\n+\t * in die().  It also avoids messing up when the pipes are dup'ed\n+\t * onto stdin/stdout/stderr in the child processes we spawn.\n+\t */\n+\tsanitize_stdfds();\n+\n \tgit_setup_gettext();\n \n \t/*\n-- \n1.8.3.2.998.g1d087bc\n"}]}