From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Sat, 27 Sep 2014 10:41:53 +0000 (+0100)
Subject: fds: Introduce pipe_cloexec()
X-Git-Tag: base.ipv6.v3~2
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?p=secnet.git;a=commitdiff_plain;h=6a06198cde5b96686304a9814dd7aa241adcb448;hp=56fd04e41e9985260b8c1660e299cfe051d3f260

fds: Introduce pipe_cloexec()

Replace all calls to pipe() with this new function, which checks
errors for us, and also sets both fds to close-on-exec.

There are some minor functional changes:
 * Error messages from pipe() failing are now less detailed about the
   context.  This is not important.
 * The signal self-pipe is now cloexec too.  This is at worst harmless.
 * When execing userv-ipif we rely on cloexec to close the spare
   copies of the pipe ends.
 * The stderr self-pipe spare writing end is redudantly made cloexec
   even though it is about to be closed shortly afterwards.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---

diff --git a/process.c b/process.c
index a9ff3d9..6468315 100644
--- a/process.c
+++ b/process.c
@@ -310,9 +310,7 @@ void start_signal_handling(void)
     sigemptyset(&registered);
     sigemptyset(&pending);
 
-    if (pipe(p)!=0) {
-	fatal_perror("start_signal_handling: pipe");
-    }
+    pipe_cloexec(p);
     spw=p[1];
     spr=p[0];
     if (fcntl(spw, F_SETFL, fcntl(spw, F_GETFL)|O_NONBLOCK)==-1) {
diff --git a/secnet.c b/secnet.c
index c604d44..7fabb1b 100644
--- a/secnet.c
+++ b/secnet.c
@@ -403,9 +403,7 @@ static void become_daemon(void)
     }
     if (secnet_is_daemon) {
 	/* stderr etc are redirected to the system/log facility */
-	if (pipe(errfds)!=0) {
-	    fatal_perror("can't create pipe for stderr");
-	}
+	pipe_cloexec(errfds);
 	if (dup2(errfds[1],0) < 0
 	    || dup2(errfds[1],1) < 0
 	    || dup2(errfds[1],2) < 0)
diff --git a/secnet.h b/secnet.h
index ce01d24..f0ab80b 100644
--- a/secnet.h
+++ b/secnet.h
@@ -139,6 +139,7 @@ extern void *safe_malloc(size_t size, const char *message);
 extern void *safe_malloc_ary(size_t size, size_t count, const char *message);
 
 void setcloexec(int fd); /* cannot fail */
+void pipe_cloexec(int fd[2]); /* pipe(), setcloexec() twice; cannot fail */
 
 extern int sys_cmd(const char *file, const char *argc, ...);
 
diff --git a/slip.c b/slip.c
index 17a9099..7d060c3 100644
--- a/slip.c
+++ b/slip.c
@@ -262,7 +262,6 @@ static void userv_entry(void *sst)
     dup2(st->in,0);
     dup2(st->out,1);
 
-    /* XXX close all other fds */
     setsid();
     /* XXX We really should strdup() all of argv[] but because we'll just
        exit anyway if execvp() fails it doesn't seem worth bothering. */
@@ -326,12 +325,8 @@ static void userv_invoke_userv(struct userv *st)
     st->slip.pending_esc=False;
 
     /* Invoke userv */
-    if (pipe(c_stdin)!=0) {
-	fatal_perror("userv_invoke_userv: pipe(c_stdin)");
-    }
-    if (pipe(c_stdout)!=0) {
-	fatal_perror("userv_invoke_userv: pipe(c_stdout)");
-    }
+    pipe_cloexec(c_stdin);
+    pipe_cloexec(c_stdout);
     st->txfd=c_stdin[1];
     st->rxfd=c_stdout[0];
 
diff --git a/util.c b/util.c
index fc6f626..aa85559 100644
--- a/util.c
+++ b/util.c
@@ -169,6 +169,13 @@ void setcloexec(int fd) {
     if (r<0) fatal_perror("fcntl(,F_SETFD,|FD_CLOEXEC) failed");
 }
 
+void pipe_cloexec(int fd[2]) {
+    int r=pipe(fd);
+    if (r) fatal_perror("pipe");
+    setcloexec(fd[0]);
+    setcloexec(fd[1]);
+}
+
 static const char *phases[NR_PHASES]={
     "PHASE_INIT",
     "PHASE_GETOPTS",