Skip to content

Commit

Permalink
make git-shell paranoid about closed stdin/stdout/stderr
Browse files Browse the repository at this point in the history
It is in general unsafe to start a program with one or more of file
descriptors 0/1/2 closed.  Karl Chen for example noticed that stat_command
does this in order to rename a pipe file descriptor to 0:

    dup2(from, 0);
    close(from);

... but if stdin was closed (for example) from == 0, so that

    dup2(0, 0);
    close(0);

just ends up closing the pipe.  Another extremely rare but nasty problem
would occur if an "important" file ends up in file descriptor 2, and is
corrupted by a call to die().

Fixing this in git was considered to be overkill, so this patch works
around it only for git-shell.  The fix is simply to open all the "low"
descriptors to /dev/null in main.

Signed-off-by: Paolo Bonzini <bonzini@gnu.org>
Acked-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Paolo Bonzini authored and Junio C Hamano committed Aug 29, 2008
1 parent 29f2815 commit 0cfeed2
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions shell.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ int main(int argc, char **argv)
{
char *prog;
struct commands *cmd;
int devnull_fd;

/*
* 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.
*/
devnull_fd = open("/dev/null", O_RDWR);
while (devnull_fd >= 0 && devnull_fd <= 2)
devnull_fd = dup(devnull_fd);
if (devnull_fd == -1)
die("opening /dev/null failed (%s)", strerror(errno));
close (devnull_fd);

/*
* Special hack to pretend to be a CVS server
Expand Down

0 comments on commit 0cfeed2

Please sign in to comment.