Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: proc.c: more syncing
To:
Tobias Heider <tobias.heider@stusta.de>
Cc:
tech@openbsd.org, florian@openbsd.org, kn@openbsd.org
Date:
Tue, 9 Apr 2024 13:56:52 +0200

Download raw body.

Thread
  • Tobias Heider:

    proc.c: more syncing

    • Alexander Bluhm:

      proc.c: more syncing

On Mon, Apr 08, 2024 at 03:34:36PM +0200, Tobias Heider wrote:
> Like in relayd, we can also drop a few dup2()s, setsid() and setpgid()
> that should be taken care of by calling daemon() earlier in proc_init().
> 
> See relayd/proc.c v1.43
> 
> ok?

OK bluhm@

> Index: sbin/iked/proc.c
> ===================================================================
> RCS file: /cvs/src/sbin/iked/proc.c,v
> diff -u -p -r1.43 proc.c
> --- sbin/iked/proc.c	8 Apr 2024 12:50:05 -0000	1.43
> +++ sbin/iked/proc.c	8 Apr 2024 13:24:23 -0000
> @@ -39,7 +39,7 @@
>  enum privsep_procid privsep_process;
>  
>  void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> -	    int, char **);
> +	    char **);
>  void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void	 proc_open(struct privsep *, int, int);
>  void	 proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -70,7 +70,7 @@ proc_getid(struct privsep_proc *procs, u
>  
>  void
>  proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -    int debug, int argc, char **argv)
> +    int argc, char **argv)
>  {
>  	unsigned int		 proc, nargc, i, proc_i;
>  	char			**nargv;
> @@ -119,10 +119,6 @@ proc_exec(struct privsep *ps, struct pri
>  				fatal("%s: fork", __func__);
>  				break;
>  			case 0:
> -				/* First create a new session */
> -				if (setsid() == -1)
> -					fatal("setsid");
> -
>  				/* Prepare parent socket. */
>  				if (fd != PROC_PARENT_SOCK_FILENO) {
>  					if (dup2(fd, PROC_PARENT_SOCK_FILENO)
> @@ -131,16 +127,6 @@ proc_exec(struct privsep *ps, struct pri
>  				} else if (fcntl(fd, F_SETFD, 0) == -1)
>  					fatal("fcntl");
>  
> -				/* Daemons detach from terminal. */
> -				if (!debug && (fd =
> -				    open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> -					(void)dup2(fd, STDIN_FILENO);
> -					(void)dup2(fd, STDOUT_FILENO);
> -					(void)dup2(fd, STDERR_FILENO);
> -					if (fd > 2)
> -						(void)close(fd);
> -				}
> -
>  				execvp(argv[0], nargv);
>  				fatal("%s: execvp", __func__);
>  				break;
> @@ -260,7 +246,7 @@ proc_init(struct privsep *ps, struct pri
>  		}
>  
>  		/* Engage! */
> -		proc_exec(ps, procs, nproc, debug, argc, argv);
> +		proc_exec(ps, procs, nproc, argc, argv);
>  		return;
>  	}
>  
> Index: usr.sbin/httpd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/proc.c,v
> diff -u -p -r1.44 proc.c
> --- usr.sbin/httpd/proc.c	8 Apr 2024 12:45:18 -0000	1.44
> +++ usr.sbin/httpd/proc.c	8 Apr 2024 13:24:23 -0000
> @@ -37,7 +37,7 @@
>  #include "httpd.h"
>  
>  void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> -	    int, char **);
> +	    char **);
>  void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void	 proc_open(struct privsep *, int, int);
>  void	 proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -68,7 +68,7 @@ proc_getid(struct privsep_proc *procs, u
>  
>  void
>  proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -    int debug, int argc, char **argv)
> +    int argc, char **argv)
>  {
>  	unsigned int		 proc, nargc, i, proc_i;
>  	char			**nargv;
> @@ -117,10 +117,6 @@ proc_exec(struct privsep *ps, struct pri
>  				fatal("%s: fork", __func__);
>  				break;
>  			case 0:
> -				/* First create a new session */
> -				if (setsid() == -1)
> -					fatal("setsid");
> -
>  				/* Prepare parent socket. */
>  				if (fd != PROC_PARENT_SOCK_FILENO) {
>  					if (dup2(fd, PROC_PARENT_SOCK_FILENO)
> @@ -129,16 +125,6 @@ proc_exec(struct privsep *ps, struct pri
>  				} else if (fcntl(fd, F_SETFD, 0) == -1)
>  					fatal("fcntl");
>  
> -				/* Daemons detach from terminal. */
> -				if (!debug && (fd =
> -				    open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> -					(void)dup2(fd, STDIN_FILENO);
> -					(void)dup2(fd, STDOUT_FILENO);
> -					(void)dup2(fd, STDERR_FILENO);
> -					if (fd > 2)
> -						(void)close(fd);
> -				}
> -
>  				execvp(argv[0], nargv);
>  				fatal("%s: execvp", __func__);
>  				break;
> @@ -232,7 +218,7 @@ proc_init(struct privsep *ps, struct pri
>  		}
>  
>  		/* Engage! */
> -		proc_exec(ps, procs, nproc, debug, argc, argv);
> +		proc_exec(ps, procs, nproc, argc, argv);
>  		return;
>  	}
>  
> @@ -525,9 +511,6 @@ proc_run(struct privsep *ps, struct priv
>  	struct control_sock	*rcs;
>  
>  	log_procinit(p->p_title);
> -
> -	/* Set the process group of the current process */
> -	setpgid(0, 0);
>  
>  	if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) {
>  		if (control_init(ps, &ps->ps_csock) == -1)
> Index: usr.sbin/vmd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
> diff -u -p -r1.24 proc.c
> --- usr.sbin/vmd/proc.c	8 Apr 2024 12:48:26 -0000	1.24
> +++ usr.sbin/vmd/proc.c	8 Apr 2024 13:24:23 -0000
> @@ -37,7 +37,7 @@
>  #include "proc.h"
>  
>  void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> -	    int, char **);
> +	    char **);
>  void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void	 proc_open(struct privsep *, int, int);
>  void	 proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -68,7 +68,7 @@ proc_getid(struct privsep_proc *procs, u
>  
>  void
>  proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -    int debug, int argc, char **argv)
> +    int argc, char **argv)
>  {
>  	unsigned int		 proc, nargc, i, proc_i;
>  	char			**nargv;
> @@ -117,10 +117,6 @@ proc_exec(struct privsep *ps, struct pri
>  				fatal("%s: fork", __func__);
>  				break;
>  			case 0:
> -				/* First create a new session */
> -				if (setsid() == -1)
> -					fatal("setsid");
> -
>  				/* Prepare parent socket. */
>  				if (fd != PROC_PARENT_SOCK_FILENO) {
>  					if (dup2(fd, PROC_PARENT_SOCK_FILENO)
> @@ -129,16 +125,6 @@ proc_exec(struct privsep *ps, struct pri
>  				} else if (fcntl(fd, F_SETFD, 0) == -1)
>  					fatal("fcntl");
>  
> -				/* Daemons detach from terminal. */
> -				if (!debug && (fd =
> -				    open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> -					(void)dup2(fd, STDIN_FILENO);
> -					(void)dup2(fd, STDOUT_FILENO);
> -					(void)dup2(fd, STDERR_FILENO);
> -					if (fd > 2)
> -						(void)close(fd);
> -				}
> -
>  				execvp(argv[0], nargv);
>  				fatal("%s: execvp", __func__);
>  				break;
> @@ -232,7 +218,7 @@ proc_init(struct privsep *ps, struct pri
>  		}
>  
>  		/* Engage! */
> -		proc_exec(ps, procs, nproc, debug, argc, argv);
> +		proc_exec(ps, procs, nproc, argc, argv);
>  		return;
>  	}
>  
> @@ -518,9 +504,6 @@ proc_run(struct privsep *ps, struct priv
>  	struct control_sock	*rcs;
>  
>  	log_procinit("%s", p->p_title);
> -
> -	/* Set the process group of the current process */
> -	setpgid(0, 0);
>  
>  	if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) {
>  		if (control_init(ps, &ps->ps_csock) == -1)
> Index: usr.sbin/snmpd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/proc.c,v
> diff -u -p -r1.31 proc.c
> --- usr.sbin/snmpd/proc.c	8 Apr 2024 13:18:54 -0000	1.31
> +++ usr.sbin/snmpd/proc.c	8 Apr 2024 13:24:23 -0000
> @@ -37,7 +37,7 @@
>  #include "snmpd.h"
>  
>  void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int, int,
> -	    int, char **);
> +	    char **);
>  void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
>  void	 proc_open(struct privsep *, int, int);
>  void	 proc_accept(struct privsep *, int, enum privsep_procid,
> @@ -68,7 +68,7 @@ proc_getid(struct privsep_proc *procs, u
>  
>  void
>  proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> -    int debug, int argc, char **argv)
> +    int argc, char **argv)
>  {
>  	unsigned int		 proc, nargc, i, proc_i;
>  	char			**nargv;
> @@ -117,10 +117,6 @@ proc_exec(struct privsep *ps, struct pri
>  				fatal("%s: fork", __func__);
>  				break;
>  			case 0:
> -				/* First create a new session */
> -				if (setsid() == -1)
> -					fatal("setsid");
> -
>  				/* Prepare parent socket. */
>  				if (fd != PROC_PARENT_SOCK_FILENO) {
>  					if (dup2(fd, PROC_PARENT_SOCK_FILENO)
> @@ -129,16 +125,6 @@ proc_exec(struct privsep *ps, struct pri
>  				} else if (fcntl(fd, F_SETFD, 0) == -1)
>  					fatal("fcntl");
>  
> -				/* Daemons detach from terminal. */
> -				if (!debug && (fd =
> -				    open(_PATH_DEVNULL, O_RDWR, 0)) != -1) {
> -					(void)dup2(fd, STDIN_FILENO);
> -					(void)dup2(fd, STDOUT_FILENO);
> -					(void)dup2(fd, STDERR_FILENO);
> -					if (fd > 2)
> -						(void)close(fd);
> -				}
> -
>  				execvp(argv[0], nargv);
>  				fatal("%s: execvp", __func__);
>  				break;
> @@ -232,7 +218,7 @@ proc_init(struct privsep *ps, struct pri
>  		}
>  
>  		/* Engage! */
> -		proc_exec(ps, procs, nproc, debug, argc, argv);
> +		proc_exec(ps, procs, nproc, argc, argv);
>  		return;
>  	}
>