close filedescriptors of children

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

close filedescriptors of children

Gerhard Roth-2
Hi,

proc_init() is done before daemon() and for the child processes of httpd,
relayd and snmpd() this function never returns. That means that the
children inherit stdin, stdout, and stderr of the caller and never close
them.

This fix this, proc_init() should map these filedes to /dev/null for a
child. The code is simpled and copied from deamon(3), without the lintish
(void) casts.

Gerhard


Index: usr.sbin/httpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.37
diff -u -p -u -p -r1.37 proc.c
--- usr.sbin/httpd/proc.c 28 May 2017 10:37:26 -0000 1.37
+++ usr.sbin/httpd/proc.c 7 Mar 2018 12:31:11 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/relayd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/proc.c,v
retrieving revision 1.39
diff -u -p -u -p -r1.39 proc.c
--- usr.sbin/relayd/proc.c 28 May 2017 10:39:15 -0000 1.39
+++ usr.sbin/relayd/proc.c 7 Mar 2018 12:32:28 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/snmpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 proc.c
--- usr.sbin/snmpd/proc.c 29 May 2017 12:56:26 -0000 1.24
+++ usr.sbin/snmpd/proc.c 7 Mar 2018 12:34:02 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");

Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Sebastian Benoit-3
Hi,

switchd and vmd use the same proc.c,and should stay in sync.

Also, this breaks -dvv (i.e. debug output when running inthe foreground),
at least for relayd.

/Benno

Gerhard Roth([hidden email]) on 2018.03.07 13:43:05 +0100:

> Hi,
>
> proc_init() is done before daemon() and for the child processes of httpd,
> relayd and snmpd() this function never returns. That means that the
> children inherit stdin, stdout, and stderr of the caller and never close
> them.
>
> This fix this, proc_init() should map these filedes to /dev/null for a
> child. The code is simpled and copied from deamon(3), without the lintish
> (void) casts.
>
> Gerhard
>
>
> Index: usr.sbin/httpd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
> retrieving revision 1.37
> diff -u -p -u -p -r1.37 proc.c
> --- usr.sbin/httpd/proc.c 28 May 2017 10:37:26 -0000 1.37
> +++ usr.sbin/httpd/proc.c 7 Mar 2018 12:31:11 -0000
> @@ -27,6 +27,7 @@
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <string.h>
> +#include <paths.h>
>  #include <errno.h>
>  #include <signal.h>
>  #include <pwd.h>
> @@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
>   unsigned int proc;
>   unsigned int dst;
>   int fds[2];
> + int fd;
>  
>   /* Don't initiate anything if we are not really going to run. */
>   if (ps->ps_noaction)
> @@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
>   fatalx("%s: process %d missing process initialization",
>      __func__, proc_id);
>  
> + if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> + dup2(fd, STDIN_FILENO);
> + dup2(fd, STDOUT_FILENO);
> + dup2(fd, STDERR_FILENO);
> + if (fd > 2)
> + close(fd);
> + }
>   p->p_init(ps, p);
>  
>   fatalx("failed to initiate child process");
> Index: usr.sbin/relayd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/proc.c,v
> retrieving revision 1.39
> diff -u -p -u -p -r1.39 proc.c
> --- usr.sbin/relayd/proc.c 28 May 2017 10:39:15 -0000 1.39
> +++ usr.sbin/relayd/proc.c 7 Mar 2018 12:32:28 -0000
> @@ -27,6 +27,7 @@
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <string.h>
> +#include <paths.h>
>  #include <errno.h>
>  #include <signal.h>
>  #include <pwd.h>
> @@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
>   unsigned int proc;
>   unsigned int dst;
>   int fds[2];
> + int fd;
>  
>   /* Don't initiate anything if we are not really going to run. */
>   if (ps->ps_noaction)
> @@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
>   fatalx("%s: process %d missing process initialization",
>      __func__, proc_id);
>  
> + if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> + dup2(fd, STDIN_FILENO);
> + dup2(fd, STDOUT_FILENO);
> + dup2(fd, STDERR_FILENO);
> + if (fd > 2)
> + close(fd);
> + }
>   p->p_init(ps, p);
>  
>   fatalx("failed to initiate child process");
> Index: usr.sbin/snmpd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
> retrieving revision 1.24
> diff -u -p -u -p -r1.24 proc.c
> --- usr.sbin/snmpd/proc.c 29 May 2017 12:56:26 -0000 1.24
> +++ usr.sbin/snmpd/proc.c 7 Mar 2018 12:34:02 -0000
> @@ -27,6 +27,7 @@
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include <string.h>
> +#include <paths.h>
>  #include <errno.h>
>  #include <signal.h>
>  #include <pwd.h>
> @@ -198,6 +199,7 @@ proc_init(struct privsep *ps, struct pri
>   unsigned int proc;
>   unsigned int dst;
>   int fds[2];
> + int fd;
>  
>   /* Don't initiate anything if we are not really going to run. */
>   if (ps->ps_noaction)
> @@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
>   fatalx("%s: process %d missing process initialization",
>      __func__, proc_id);
>  
> + if ((fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> + dup2(fd, STDIN_FILENO);
> + dup2(fd, STDOUT_FILENO);
> + dup2(fd, STDERR_FILENO);
> + if (fd > 2)
> + close(fd);
> + }
>   p->p_init(ps, p);
>  
>   fatalx("failed to initiate child process");
>

Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Gerhard Roth-2
Hi Benno,

thanks for your reply.

On Wed, 7 Mar 2018 15:22:28 +0100 Sebastian Benoit <[hidden email]> wrote:
> Hi,
>
> switchd and vmd use the same proc.c,and should stay in sync.

Ack. I missed them.


>
> Also, this breaks -dvv (i.e. debug output when running inthe foreground),
> at least for relayd.

Stupid me, indeed.

>
> /Benno
>

Below is an updated patch that includes proc.c of switchd and vmd.
It also passes the 'debug' flag to proc_init() so that it won't touch
std* in that case.

I was wandering if this would be a good idea to move the daemon(3)
call into the PROC_PARENT case of proc_init(), too?


Gerhard


Index: usr.sbin/httpd/httpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
retrieving revision 1.67
diff -u -p -u -p -r1.67 httpd.c
--- usr.sbin/httpd/httpd.c 28 May 2017 10:37:26 -0000 1.67
+++ usr.sbin/httpd/httpd.c 7 Mar 2018 15:49:47 -0000
@@ -215,7 +215,7 @@ main(int argc, char *argv[])
  }
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  log_procinit("parent");
  if (!debug && daemon(1, 0) == -1)
Index: usr.sbin/httpd/httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.135
diff -u -p -u -p -r1.135 httpd.h
--- usr.sbin/httpd/httpd.h 7 Feb 2018 03:28:05 -0000 1.135
+++ usr.sbin/httpd/httpd.h 7 Mar 2018 15:49:30 -0000
@@ -761,7 +761,7 @@ __dead void fatalx(const char *, ...)
 enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/httpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.37
diff -u -p -u -p -r1.37 proc.c
--- usr.sbin/httpd/proc.c 28 May 2017 10:37:26 -0000 1.37
+++ usr.sbin/httpd/proc.c 7 Mar 2018 15:50:06 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -191,13 +192,14 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if (!debug && (fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/relayd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/proc.c,v
retrieving revision 1.39
diff -u -p -u -p -r1.39 proc.c
--- usr.sbin/relayd/proc.c 28 May 2017 10:39:15 -0000 1.39
+++ usr.sbin/relayd/proc.c 7 Mar 2018 15:43:03 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -191,13 +192,14 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if (!debug && (fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.171
diff -u -p -u -p -r1.171 relayd.c
--- usr.sbin/relayd/relayd.c 29 Nov 2017 15:24:50 -0000 1.171
+++ usr.sbin/relayd/relayd.c 7 Mar 2018 15:42:24 -0000
@@ -212,7 +212,7 @@ main(int argc, char *argv[])
  ps->ps_title[proc_id] = title;
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  log_procinit("parent");
  if (!debug && daemon(1, 0) == -1)
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.248
diff -u -p -u -p -r1.248 relayd.h
--- usr.sbin/relayd/relayd.h 28 Nov 2017 18:25:53 -0000 1.248
+++ usr.sbin/relayd/relayd.h 7 Mar 2018 15:42:34 -0000
@@ -1383,7 +1383,7 @@ enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 int proc_flush_imsg(struct privsep *, enum privsep_procid, int);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/snmpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 proc.c
--- usr.sbin/snmpd/proc.c 29 May 2017 12:56:26 -0000 1.24
+++ usr.sbin/snmpd/proc.c 7 Mar 2018 15:48:23 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -191,13 +192,14 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if (!debug && (fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/snmpd/snmpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
retrieving revision 1.37
diff -u -p -u -p -r1.37 snmpd.c
--- usr.sbin/snmpd/snmpd.c 12 Aug 2017 04:29:57 -0000 1.37
+++ usr.sbin/snmpd/snmpd.c 7 Mar 2018 15:48:41 -0000
@@ -230,7 +230,7 @@ main(int argc, char *argv[])
  pf_init();
  snmpd_generate_engineid(env);
 
- proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id, debug);
  if (!debug && daemon(0, 0) == -1)
  err(1, "failed to daemonize");
 
Index: usr.sbin/snmpd/snmpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.77
diff -u -p -u -p -r1.77 snmpd.h
--- usr.sbin/snmpd/snmpd.h 8 Feb 2018 00:21:10 -0000 1.77
+++ usr.sbin/snmpd/snmpd.h 7 Mar 2018 15:45:01 -0000
@@ -759,7 +759,7 @@ void usm_make_report(struct snmp_messa
 enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/switchd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.c,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 proc.c
--- usr.sbin/switchd/proc.c 29 May 2017 12:56:26 -0000 1.12
+++ usr.sbin/switchd/proc.c 7 Mar 2018 15:52:24 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -191,13 +192,14 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if (!debug && (fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/switchd/proc.h
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.h,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 proc.h
--- usr.sbin/switchd/proc.h 9 Jan 2017 14:49:22 -0000 1.6
+++ usr.sbin/switchd/proc.h 7 Mar 2018 15:51:16 -0000
@@ -127,7 +127,7 @@ extern  struct ctl_connlist ctl_conns;
 
 /* proc.c */
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *ps);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/switchd/switchd.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/switchd.c,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 switchd.c
--- usr.sbin/switchd/switchd.c 9 Jan 2017 14:49:22 -0000 1.15
+++ usr.sbin/switchd/switchd.c 7 Mar 2018 15:51:22 -0000
@@ -184,7 +184,7 @@ main(int argc, char *argv[])
  ps->ps_title[proc_id] = title;
 
  /* Only the parent returns. */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  if (!debug && daemon(0, 0) == -1)
  fatal("failed to daemonize");
Index: usr.sbin/vmd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 proc.c
--- usr.sbin/vmd/proc.c 4 Nov 2017 07:40:31 -0000 1.16
+++ usr.sbin/vmd/proc.c 7 Mar 2018 15:56:13 -0000
@@ -27,6 +27,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <string.h>
+#include <paths.h>
 #include <errno.h>
 #include <signal.h>
 #include <pwd.h>
@@ -191,13 +192,14 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
  unsigned int proc;
  unsigned int dst;
  int fds[2];
+ int fd;
 
  /* Don't initiate anything if we are not really going to run. */
  if (ps->ps_noaction)
@@ -246,6 +248,13 @@ proc_init(struct privsep *ps, struct pri
  fatalx("%s: process %d missing process initialization",
     __func__, proc_id);
 
+ if (!debug && (fd = open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
+ dup2(fd, STDIN_FILENO);
+ dup2(fd, STDOUT_FILENO);
+ dup2(fd, STDERR_FILENO);
+ if (fd > 2)
+ close(fd);
+ }
  p->p_init(ps, p);
 
  fatalx("failed to initiate child process");
Index: usr.sbin/vmd/proc.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/proc.h,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 proc.h
--- usr.sbin/vmd/proc.h 27 Mar 2017 00:28:04 -0000 1.12
+++ usr.sbin/vmd/proc.h 7 Mar 2018 15:53:14 -0000
@@ -159,7 +159,7 @@ struct privsep_fd {
 
 /* proc.c */
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *ps);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/vmd/vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.80
diff -u -p -u -p -r1.80 vmd.c
--- usr.sbin/vmd/vmd.c 18 Feb 2018 01:00:25 -0000 1.80
+++ usr.sbin/vmd/vmd.c 7 Mar 2018 15:55:03 -0000
@@ -755,7 +755,8 @@ main(int argc, char **argv)
  ps->ps_title[proc_id] = title;
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id,
+    env->vmd_debug);
 
  log_procinit("parent");
  if (!env->vmd_debug && daemon(0, 0) == -1)

Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Mike Belopuhov-5
On 7 March 2018 at 17:01, Gerhard Roth <[hidden email]> wrote:
>
> Hi Benno,
>
> thanks for your reply.
>
> On Wed, 7 Mar 2018 15:22:28 +0100 Sebastian Benoit <[hidden email]>
wrote:
> > Hi,
> >
> > switchd and vmd use the same proc.c,and should stay in sync.
>
> Ack. I missed them.
>

iked also uses proc.c. I think you've got all the others,
but perhaps you should run a find?

Cheers,
Mike
Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Gerhard Roth-2
On Wed, 7 Mar 2018 17:20:06 +0100 Mike Belopuhov <[hidden email]> wrote:

> On 7 March 2018 at 17:01, Gerhard Roth <[hidden email]> wrote:
> >
> > Hi Benno,
> >
> > thanks for your reply.
> >
> > On Wed, 7 Mar 2018 15:22:28 +0100 Sebastian Benoit <[hidden email]>  
> wrote:
> > > Hi,
> > >
> > > switchd and vmd use the same proc.c,and should stay in sync.  
> >
> > Ack. I missed them.
> >  
>
> iked also uses proc.c. I think you've got all the others,
> but perhaps you should run a find?
>
> Cheers,
> Mike

Hi Mike,

but iked still uses an older version of proc.c that just forks off
the children but does not execve() the own binary.

Also, iked is the only one that daemon(3)-izes before calling
proc_init(). So here stdout, stdin, and stderr is already remapped
to /dev/null before forking the kids.

Gerhard

Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Mike Belopuhov-5
On 7 March 2018 at 17:27, Gerhard Roth <[hidden email]> wrote:
>
> On Wed, 7 Mar 2018 17:20:06 +0100 Mike Belopuhov <[hidden email]>
wrote:

> > On 7 March 2018 at 17:01, Gerhard Roth <[hidden email]> wrote:
> > >
> > > Hi Benno,
> > >
> > > thanks for your reply.
> > >
> > > On Wed, 7 Mar 2018 15:22:28 +0100 Sebastian Benoit <[hidden email]>
> > wrote:
> > > > Hi,
> > > >
> > > > switchd and vmd use the same proc.c,and should stay in sync.
> > >
> > > Ack. I missed them.
> > >
> >
> > iked also uses proc.c. I think you've got all the others,
> > but perhaps you should run a find?
> >
> > Cheers,
> > Mike
>
> Hi Mike,
>
> but iked still uses an older version of proc.c that just forks off
> the children but does not execve() the own binary.
>
> Also, iked is the only one that daemon(3)-izes before calling
> proc_init(). So here stdout, stdin, and stderr is already remapped
> to /dev/null before forking the kids.
>
> Gerhard

I see.  Reyk always wanted to keep them all in sync, but I guess
it's too late to care about that if they've already diverged.
Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Damien Miller
In reply to this post by Gerhard Roth-2
On Wed, 7 Mar 2018, Gerhard Roth wrote:

> Below is an updated patch that includes proc.c of switchd and vmd.
> It also passes the 'debug' flag to proc_init() so that it won't touch
> std* in that case.

FWIW sshd unconditionally clobbers stdin and stdout and will also
clobber stderr if the debug flag isn't set.

-d

Reply | Threaded
Open this post in threaded view
|

Re: close filedescriptors of children

Gerhard Roth-2
In reply to this post by Mike Belopuhov-5
If proc_init() knows about debug mode, we can move the call to daemon(3)
into proc_init(). Then only the parent calls daemon(3). The children will
inherit stdin/out/err from the parent and don't have to do anything.
And since the children don't call daemon(3) themselves anymore, there
won't be any zombies.

Below is an updated, more simpler patch.

The only problem here is that half of the daemons set the 'nochdir'
flag of daemon(3), while the other half doesn't. Hence the proc.c files
differ in the arguments passed to daemon(3). If this is a problem, I
can make this a parameter passed to proc_init().


Gerhard


Index: usr.sbin/httpd/httpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.c,v
retrieving revision 1.67
diff -u -p -u -p -r1.67 httpd.c
--- usr.sbin/httpd/httpd.c 28 May 2017 10:37:26 -0000 1.67
+++ usr.sbin/httpd/httpd.c 8 Mar 2018 09:17:07 -0000
@@ -215,11 +215,9 @@ main(int argc, char *argv[])
  }
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  log_procinit("parent");
- if (!debug && daemon(1, 0) == -1)
- err(1, "failed to daemonize");
 
  if (ps->ps_noaction == 0)
  log_info("startup");
Index: usr.sbin/httpd/httpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/httpd.h,v
retrieving revision 1.135
diff -u -p -u -p -r1.135 httpd.h
--- usr.sbin/httpd/httpd.h 7 Feb 2018 03:28:05 -0000 1.135
+++ usr.sbin/httpd/httpd.h 7 Mar 2018 15:49:30 -0000
@@ -761,7 +761,7 @@ __dead void fatalx(const char *, ...)
 enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/httpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
retrieving revision 1.37
diff -u -p -u -p -r1.37 proc.c
--- usr.sbin/httpd/proc.c 28 May 2017 10:37:26 -0000 1.37
+++ usr.sbin/httpd/proc.c 8 Mar 2018 09:17:46 -0000
@@ -191,7 +191,7 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
@@ -205,6 +205,8 @@ proc_init(struct privsep *ps, struct pri
 
  if (proc_id == PROC_PARENT) {
  privsep_process = PROC_PARENT;
+ if (!debug && daemon(1, 0) == -1)
+ fatal("failed to daemonize");
  proc_setup(ps, procs, nproc);
 
  /*
Index: usr.sbin/relayd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/proc.c,v
retrieving revision 1.39
diff -u -p -u -p -r1.39 proc.c
--- usr.sbin/relayd/proc.c 28 May 2017 10:39:15 -0000 1.39
+++ usr.sbin/relayd/proc.c 8 Mar 2018 09:19:41 -0000
@@ -191,7 +191,7 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
@@ -205,6 +205,8 @@ proc_init(struct privsep *ps, struct pri
 
  if (proc_id == PROC_PARENT) {
  privsep_process = PROC_PARENT;
+ if (!debug && daemon(1, 0) == -1)
+ fatal("failed to daemonize");
  proc_setup(ps, procs, nproc);
 
  /*
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.171
diff -u -p -u -p -r1.171 relayd.c
--- usr.sbin/relayd/relayd.c 29 Nov 2017 15:24:50 -0000 1.171
+++ usr.sbin/relayd/relayd.c 8 Mar 2018 09:18:21 -0000
@@ -212,11 +212,9 @@ main(int argc, char *argv[])
  ps->ps_title[proc_id] = title;
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  log_procinit("parent");
- if (!debug && daemon(1, 0) == -1)
- err(1, "failed to daemonize");
 
  if (ps->ps_noaction == 0)
  log_info("startup");
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.248
diff -u -p -u -p -r1.248 relayd.h
--- usr.sbin/relayd/relayd.h 28 Nov 2017 18:25:53 -0000 1.248
+++ usr.sbin/relayd/relayd.h 7 Mar 2018 15:42:34 -0000
@@ -1383,7 +1383,7 @@ enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 int proc_flush_imsg(struct privsep *, enum privsep_procid, int);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/snmpd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
retrieving revision 1.24
diff -u -p -u -p -r1.24 proc.c
--- usr.sbin/snmpd/proc.c 29 May 2017 12:56:26 -0000 1.24
+++ usr.sbin/snmpd/proc.c 8 Mar 2018 09:20:40 -0000
@@ -191,7 +191,7 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
@@ -205,6 +205,8 @@ proc_init(struct privsep *ps, struct pri
 
  if (proc_id == PROC_PARENT) {
  privsep_process = PROC_PARENT;
+ if (!debug && daemon(0, 0) == -1)
+ fatal("failed to daemonize");
  proc_setup(ps, procs, nproc);
 
  /*
Index: usr.sbin/snmpd/snmpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.c,v
retrieving revision 1.37
diff -u -p -u -p -r1.37 snmpd.c
--- usr.sbin/snmpd/snmpd.c 12 Aug 2017 04:29:57 -0000 1.37
+++ usr.sbin/snmpd/snmpd.c 8 Mar 2018 09:20:13 -0000
@@ -230,9 +230,7 @@ main(int argc, char *argv[])
  pf_init();
  snmpd_generate_engineid(env);
 
- proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id);
- if (!debug && daemon(0, 0) == -1)
- err(1, "failed to daemonize");
+ proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id, debug);
 
  log_procinit("parent");
  log_info("startup");
Index: usr.sbin/snmpd/snmpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.77
diff -u -p -u -p -r1.77 snmpd.h
--- usr.sbin/snmpd/snmpd.h 8 Feb 2018 00:21:10 -0000 1.77
+++ usr.sbin/snmpd/snmpd.h 7 Mar 2018 15:45:01 -0000
@@ -759,7 +759,7 @@ void usm_make_report(struct snmp_messa
 enum privsep_procid
     proc_getid(struct privsep_proc *, unsigned int, const char *);
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/switchd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.c,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 proc.c
--- usr.sbin/switchd/proc.c 29 May 2017 12:56:26 -0000 1.12
+++ usr.sbin/switchd/proc.c 8 Mar 2018 09:21:41 -0000
@@ -191,7 +191,7 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
@@ -205,6 +205,8 @@ proc_init(struct privsep *ps, struct pri
 
  if (proc_id == PROC_PARENT) {
  privsep_process = PROC_PARENT;
+ if (!debug && daemon(0, 0) == -1)
+ fatal("failed to daemonize");
  proc_setup(ps, procs, nproc);
 
  /*
Index: usr.sbin/switchd/proc.h
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/proc.h,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 proc.h
--- usr.sbin/switchd/proc.h 9 Jan 2017 14:49:22 -0000 1.6
+++ usr.sbin/switchd/proc.h 7 Mar 2018 15:51:16 -0000
@@ -127,7 +127,7 @@ extern  struct ctl_connlist ctl_conns;
 
 /* proc.c */
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *ps);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/switchd/switchd.c
===================================================================
RCS file: /cvs/src/usr.sbin/switchd/switchd.c,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 switchd.c
--- usr.sbin/switchd/switchd.c 9 Jan 2017 14:49:22 -0000 1.15
+++ usr.sbin/switchd/switchd.c 8 Mar 2018 09:21:09 -0000
@@ -184,10 +184,7 @@ main(int argc, char *argv[])
  ps->ps_title[proc_id] = title;
 
  /* Only the parent returns. */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
-
- if (!debug && daemon(0, 0) == -1)
- fatal("failed to daemonize");
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id, debug);
 
  log_procinit("parent");
 
Index: usr.sbin/vmd/proc.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 proc.c
--- usr.sbin/vmd/proc.c 4 Nov 2017 07:40:31 -0000 1.16
+++ usr.sbin/vmd/proc.c 8 Mar 2018 09:25:05 -0000
@@ -191,7 +191,7 @@ proc_connect(struct privsep *ps)
 
 void
 proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
-    int argc, char **argv, enum privsep_procid proc_id)
+    int argc, char **argv, enum privsep_procid proc_id, int debug)
 {
  struct privsep_proc *p = NULL;
  struct privsep_pipes *pa, *pb;
@@ -205,6 +205,8 @@ proc_init(struct privsep *ps, struct pri
 
  if (proc_id == PROC_PARENT) {
  privsep_process = PROC_PARENT;
+ if (!env->vmd_debug && daemon(0, 0) == -1)
+ fatal("failed to daemonize");
  proc_setup(ps, procs, nproc);
 
  /*
Index: usr.sbin/vmd/proc.h
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/proc.h,v
retrieving revision 1.12
diff -u -p -u -p -r1.12 proc.h
--- usr.sbin/vmd/proc.h 27 Mar 2017 00:28:04 -0000 1.12
+++ usr.sbin/vmd/proc.h 7 Mar 2018 15:53:14 -0000
@@ -159,7 +159,7 @@ struct privsep_fd {
 
 /* proc.c */
 void proc_init(struct privsep *, struct privsep_proc *, unsigned int,
-    int, char **, enum privsep_procid);
+    int, char **, enum privsep_procid, int);
 void proc_kill(struct privsep *);
 void proc_connect(struct privsep *ps);
 void proc_dispatch(int, short event, void *);
Index: usr.sbin/vmd/vmd.c
===================================================================
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.80
diff -u -p -u -p -r1.80 vmd.c
--- usr.sbin/vmd/vmd.c 18 Feb 2018 01:00:25 -0000 1.80
+++ usr.sbin/vmd/vmd.c 8 Mar 2018 09:22:16 -0000
@@ -755,11 +755,10 @@ main(int argc, char **argv)
  ps->ps_title[proc_id] = title;
 
  /* only the parent returns */
- proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
+ proc_init(ps, procs, nitems(procs), argc0, argv, proc_id,
+    env->vmd_debug);
 
  log_procinit("parent");
- if (!env->vmd_debug && daemon(0, 0) == -1)
- fatal("can't daemonize");
 
  if (ps->ps_noaction == 0)
  log_info("startup");