ok, comments below

Reyk Floeter([email protected]) on 2016.09.02 19:03:47 +0200:
> Hi,
> 
> after all the preparation, the following diff adds support for
> fork+exec in relayd; based on rzalamena@'s work for httpd.
> 
> Notes:
> - proc.c is identical to the version that is already in httpd.
> - The recvfd pledge in some procs is only needed once, could be
> dropped later after receiving IMSG_CTL_PROCFD.
> - The socket_rlimit(-1) in the parent is needed because relayd
> currently opens n:m fds between all processes which slightly exceeds
> the allowed 128 fds.  The problem also exists in httpd and in -current
> but the relayd fork+exec diff needs a few more fds and gets over the
> limit.  The problem will be reviewed and fixed independently.
> - Don't forget to clean and rebuild relayctl.
> 
> All regress tests pass, but it needs some more testing.
> 
> OK?
> 
> Reyk
> 
> Index: usr.sbin/relayd/ca.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
> retrieving revision 1.21
> diff -u -p -u -p -r1.21 ca.c
> --- usr.sbin/relayd/ca.c      2 Sep 2016 14:45:51 -0000       1.21
> +++ usr.sbin/relayd/ca.c      2 Sep 2016 16:49:53 -0000
> @@ -73,7 +73,7 @@ ca(struct privsep *ps, struct privsep_pr
>  void
>  ca_init(struct privsep *ps, struct privsep_proc *p, void *arg)
>  {
> -     if (pledge("stdio", NULL) == -1)
> +     if (pledge("stdio recvfd", NULL) == -1)
>               fatal("pledge");
>  
>       if (config_init(ps->ps_env) == -1)
> Index: usr.sbin/relayd/hce.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/hce.c,v
> retrieving revision 1.74
> diff -u -p -u -p -r1.74 hce.c
> --- usr.sbin/relayd/hce.c     2 Sep 2016 14:45:51 -0000       1.74
> +++ usr.sbin/relayd/hce.c     2 Sep 2016 16:49:53 -0000
> @@ -70,7 +70,7 @@ hce_init(struct privsep *ps, struct priv
>       /* Allow maximum available sockets for TCP checks */
>       socket_rlimit(-1);
>  
> -     if (pledge("stdio inet", NULL) == -1)
> +     if (pledge("stdio recvfd inet", NULL) == -1)
>               fatal("hce: pledge");
>  }
>  
> Index: usr.sbin/relayd/relayd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
> retrieving revision 1.159
> diff -u -p -u -p -r1.159 relayd.c
> --- usr.sbin/relayd/relayd.c  2 Sep 2016 14:45:51 -0000       1.159
> +++ usr.sbin/relayd/relayd.c  2 Sep 2016 16:49:54 -0000
> @@ -121,8 +121,12 @@ main(int argc, char *argv[])
>       struct relayd           *env;
>       struct privsep          *ps;
>       const char              *conffile = CONF_FILE;
> +     enum privsep_procid      proc_id = PROC_PARENT;
> +     int                      proc_instance = 0;
> +     const char              *errp, *title = NULL;
> +     int                      argc0 = argc;
>  
> -     while ((c = getopt(argc, argv, "dD:nf:v")) != -1) {
> +     while ((c = getopt(argc, argv, "dD:nI:P:f:v")) != -1) {
>               switch (c) {
>               case 'd':
>                       debug = 2;
> @@ -143,6 +147,18 @@ main(int argc, char *argv[])
>                       verbose++;
>                       opts |= RELAYD_OPT_VERBOSE;
>                       break;
> +             case 'P':
> +                     title = optarg;
> +                     proc_id = proc_getid(procs, nitems(procs), title);
> +                     if (proc_id == PROC_MAX)
> +                             fatalx("invalid process name");
> +                     break;
> +             case 'I':
> +                     proc_instance = strtonum(optarg, 0,
> +                         PROC_MAX_INSTANCES, &errp);
> +                     if (errp)
> +                             fatalx("invalid process instance");
> +                     break;
>               default:
>                       usage();
>               }
> @@ -189,19 +205,29 @@ main(int argc, char *argv[])
>       log_init(debug, LOG_DAEMON);
>       log_verbose(verbose);
>  
> -     if (!debug && daemon(1, 0) == -1)
> -             err(1, "failed to daemonize");
> -
>       if (env->sc_conf.opts & RELAYD_OPT_NOACTION)
>               ps->ps_noaction = 1;
> -     else
> -             log_info("startup");
>  
>       ps->ps_instances[PROC_RELAY] = env->sc_conf.prefork_relay;
>       ps->ps_instances[PROC_CA] = env->sc_conf.prefork_relay;
> +     ps->ps_instance = proc_instance;
> +     if (title != NULL)
> +             ps->ps_title[proc_id] = title;
> +
> +     if (proc_id == PROC_PARENT) {
> +             /* XXX the parent opens too many fds in proc_open() */
> +             socket_rlimit(-1);
> +     }
> +
> +     /* only the parent returns */
> +     proc_init(ps, procs, nitems(procs), argc0, argv, proc_id);
>  
> -     proc_init(ps, procs, nitems(procs));
>       log_procinit("parent");
> +     if (!debug && daemon(1, 0) == -1)
> +             err(1, "failed to daemonize");
> +
> +     if (ps->ps_noaction == 0)
> +             log_info("startup");
>  
>       event_init();
>  
> @@ -217,7 +243,7 @@ main(int argc, char *argv[])
>       signal_add(&ps->ps_evsigpipe, NULL);
>       signal_add(&ps->ps_evsigusr1, NULL);
>  
> -     proc_listen(ps, procs, nitems(procs));
> +     proc_connect(ps);
>  
>       if (load_config(env->sc_conffile, env) == -1) {
>               proc_kill(env->sc_ps);
> Index: usr.sbin/relayd/relayd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
> retrieving revision 1.231
> diff -u -p -u -p -r1.231 relayd.h
> --- usr.sbin/relayd/relayd.h  2 Sep 2016 16:14:09 -0000       1.231
> +++ usr.sbin/relayd/relayd.h  2 Sep 2016 16:49:54 -0000
> @@ -915,6 +915,7 @@ enum imsg_type {
>       IMSG_CTL_OK,            /* answer to relayctl requests */
>       IMSG_CTL_FAIL,
>       IMSG_CTL_VERBOSE,
> +     IMSG_CTL_PROCFD,
>       IMSG_CTL_END,
>       IMSG_CTL_RDR,
>       IMSG_CTL_TABLE,
> @@ -987,6 +988,11 @@ enum privsep_procid {
>  /* Attach the control socket to the following process */
>  #define PROC_CONTROL PROC_PFE
>  
> +/* Define default parent socket number */
> +#define PARENT_SOCK_FILENO   3
> +
> +#define PROC_MAX_INSTANCES   128
> +
>  struct privsep_pipes {
>       int                             *pp_pipes[PROC_MAX];
>  };
> @@ -1031,6 +1037,11 @@ struct privsep_proc {
>       struct relayd           *p_env;
>  };
>  
> +struct privsep_fd {
> +     enum privsep_procid              pf_procid;
> +     unsigned int                     pf_instance;
> +};
> +
>  struct relayd_config {
>       char                     tls_sid[SSL_MAX_SID_CTX_LENGTH];
>       char                     snmp_path[PATH_MAX];
> @@ -1363,12 +1374,15 @@ __dead void fatalx(const char *, ...)
>           __attribute__((__format__ (printf, 1, 2)));
>  
>  /* proc.c */
> -void  proc_init(struct privsep *, struct privsep_proc *, u_int);
> +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);
>  void  proc_kill(struct privsep *);
> -void  proc_listen(struct privsep *, struct privsep_proc *, size_t);
> +void  proc_connect(struct privsep *);
>  void  proc_dispatch(int, short event, void *);
>  void  proc_run(struct privsep *, struct privsep_proc *,
> -         struct privsep_proc *, u_int,
> +         struct privsep_proc *, unsigned int,
>           void (*)(struct privsep *, struct privsep_proc *, void *), void *);
>  void  proc_range(struct privsep *, enum privsep_procid, int *, int *);
>  int   proc_compose_imsg(struct privsep *, enum privsep_procid, int,
> @@ -1377,18 +1391,18 @@ int    proc_compose(struct privsep *, enum
>           uint16_t, void *, uint16_t);
>  int   proc_composev_imsg(struct privsep *, enum privsep_procid, int,
>           u_int16_t, u_int32_t, int, const struct iovec *, int);
> -int   proc_forward_imsg(struct privsep *, struct imsg *,
> -         enum privsep_procid, int);
>  int   proc_composev(struct privsep *, enum privsep_procid,
>           uint16_t, const struct iovec *, int);
> +int   proc_forward_imsg(struct privsep *, struct imsg *,
> +         enum privsep_procid, int);
>  struct imsgbuf *
>        proc_ibuf(struct privsep *, enum privsep_procid, int);
>  struct imsgev *
>        proc_iev(struct privsep *, enum privsep_procid, int);
>  void  imsg_event_add(struct imsgev *);
> -int   imsg_compose_event(struct imsgev *, u_int16_t, u_int32_t,
> -         pid_t, int, void *, u_int16_t);
> -int   imsg_composev_event(struct imsgev *, u_int16_t, u_int32_t,
> +int   imsg_compose_event(struct imsgev *, uint16_t, uint32_t,
> +         pid_t, int, void *, uint16_t);
> +int   imsg_composev_event(struct imsgev *, uint16_t, uint32_t,
>           pid_t, int, const struct iovec *, int);
>  
>  /* config.c */
> Index: usr.sbin/relayd/proc.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/proc.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 proc.c
> --- usr.sbin/relayd/proc.c    2 Sep 2016 12:14:08 -0000       1.30
> +++ usr.sbin/relayd/proc.c    2 Sep 2016 16:49:54 -0000
> @@ -1,4 +1,4 @@
> -/*   $OpenBSD: proc.c,v 1.30 2016/09/02 12:14:08 reyk Exp $  */
> +/*   $OpenBSD: proc.c,v 1.24 2016/09/02 11:25:14 reyk Exp $  */
>  
>  /*
>   * Copyright (c) 2010 - 2014 Reyk Floeter <[email protected]>
> @@ -34,8 +34,14 @@
>  
>  #include "relayd.h"
>  
> -void  proc_open(struct privsep *, struct privsep_proc *,
> -         struct privsep_proc *, size_t);
> +void  proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
> +         int, char **);
> +void  proc_connectpeer(struct privsep *, enum privsep_procid, int,
> +         struct privsep_pipes *);
> +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,
> +         unsigned int);
>  void  proc_close(struct privsep *);
>  int   proc_ispeer(struct privsep_proc *, unsigned int, enum privsep_procid);
>  void  proc_shutdown(struct privsep_proc *);
> @@ -55,17 +61,264 @@ proc_ispeer(struct privsep_proc *procs, 
>       return (0);
>  }
>  
> +enum privsep_procid
> +proc_getid(struct privsep_proc *procs, unsigned int nproc,
> +    const char *proc_name)
> +{
> +     struct privsep_proc     *p;
> +     unsigned int             proc;
> +
> +     for (proc = 0; proc < nproc; proc++) {
> +             p = &procs[proc];
> +             if (strcmp(p->p_title, proc_name))
> +                     continue;
> +
> +             return (p->p_id);
> +     }
> +
> +     return (PROC_MAX);
> +}
> +
> +void
> +proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> +    int argc, char **argv)
> +{
> +     unsigned int             proc, nargc, i, proc_i;
> +     char                    **nargv;
> +     struct privsep_proc     *p;
> +     char                     num[32];
> +     int                      fd;
> +
> +     /* Prepare the new process argv. */
> +     nargv = calloc(argc + 5, sizeof(char *));
> +     if (nargv == NULL)
> +             fatal("%s: calloc", __func__);
> +
> +     /* Copy call argument first. */
> +     nargc = 0;
> +     nargv[nargc++] = argv[0];
> +
> +     /* Set process name argument and save the position. */
> +     nargv[nargc++] = "-P";
> +     proc_i = nargc;
> +     nargc++;
> +
> +     /* Point process instance arg to stack and copy the original args. */
> +     nargv[nargc++] = "-I";
> +     nargv[nargc++] = num;
> +     for (i = 1; i < (unsigned int) argc; i++)
> +             nargv[nargc++] = argv[i];
> +
> +     nargv[nargc] = NULL;
> +
> +     for (proc = 0; proc < nproc; proc++) {
> +             p = &procs[proc];
> +
> +             /* Update args with process title. */
> +             nargv[proc_i] = (char *) p->p_title;
> +
> +             /* Fire children processes. */
> +             for (i = 0; i < ps->ps_instances[p->p_id]; i++) {
> +                     /* Update the process instance number. */
> +                     snprintf(num, sizeof(num), "%u", i);
> +
> +                     fd = ps->ps_pipes[p->p_id][i].pp_pipes[PROC_PARENT][0];
> +                     ps->ps_pipes[p->p_id][i].pp_pipes[PROC_PARENT][0] = -1;
> +
> +                     switch (fork()) {
> +                     case -1:
> +                             fatal("%s: fork", __func__);
> +                             break;
> +                     case 0:
> +                             /* Prepare parent socket. */
> +                             dup2(fd, PARENT_SOCK_FILENO);
> +
> +                             execvp(argv[0], nargv);
> +                             fatal("%s: execvp", __func__);
> +                             break;
> +                     default:
> +                             /* Close child end. */
> +                             close(fd);
> +                             break;
> +                     }
> +             }
> +     }
> +     free(nargv);
> +}
> +
> +void
> +proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
> +    struct privsep_pipes *pp)
> +{
> +     unsigned int             i, j;
> +     struct privsep_fd        pf;
> +
> +     for (i = 0; i < PROC_MAX; i++) {
> +             /* Parent is already connected with everyone. */
> +             if (i == PROC_PARENT)
> +                     continue;
> +
> +             for (j = 0; j < ps->ps_instances[i]; j++) {
> +                     /* Don't send socket to child itself. */
> +                     if (i == (unsigned int)id &&
> +                         j == (unsigned int)inst)
> +                             continue;
> +                     if (pp->pp_pipes[i][j] == -1)
> +                             continue;
> +
> +                     pf.pf_procid = i;
> +                     pf.pf_instance = j;
> +                     proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
> +                         -1, pp->pp_pipes[i][j], &pf, sizeof(pf));
> +                     pp->pp_pipes[i][j] = -1;
> +             }
> +     }
> +}
> +
> +/* Inter-connect all process except with ourself. */
> +void
> +proc_connect(struct privsep *ps)
> +{
> +     unsigned int             src, i, j;
> +     struct privsep_pipes    *pp;
> +     struct imsgev           *iev;
> +
> +     /* Listen on appropriate pipes. */
> +     src = privsep_process;
> +     pp = &ps->ps_pipes[src][ps->ps_instance];
> +
> +     for (i = 0; i < PROC_MAX; i++) {
> +             /* Don't listen to ourself. */
> +             if (i == src)
> +                     continue;
> +
> +             for (j = 0; j < ps->ps_instances[i]; j++) {
> +                     if (pp->pp_pipes[i][j] == -1)
> +                             continue;
> +
> +                     iev = &ps->ps_ievs[i][j];
> +                     imsg_init(&iev->ibuf, pp->pp_pipes[i][j]);
> +                     event_set(&iev->ev, iev->ibuf.fd, iev->events,
> +                         iev->handler, iev->data);
> +                     event_add(&iev->ev, NULL);
> +             }
> +     }
> +
> +     /* Exchange pipes between process. */
> +     for (i = 0; i < PROC_MAX; i++) {
> +             /* Parent is already connected with everyone. */
> +             if (i == PROC_PARENT)
> +                     continue;
> +
> +             for (j = 0; j < ps->ps_instances[i]; j++)
> +                     proc_connectpeer(ps, i, j, &ps->ps_pipes[i][j]);
> +     }
> +}
> +
> +void
> +proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
> +    int argc, char **argv, enum privsep_procid proc_id)
> +{
> +     struct privsep_proc     *p = NULL;
> +     unsigned int             proc;
> +     unsigned int             src, dst;
> +
> +     if (proc_id == PROC_PARENT) {
> +             privsep_process = PROC_PARENT;
> +             proc_setup(ps, procs, nproc);
> +
> +             /* Open socketpair()s for everyone. */
> +             for (src = 0; src < PROC_MAX; src++)
> +                     for (dst = 0; dst < PROC_MAX; dst++)
> +                             proc_open(ps, src, dst);
> +
> +             /* Engage! */
> +             proc_exec(ps, procs, nproc, argc, argv);
> +             return;
> +     }
> +
> +     /* Initialize a child */
> +     for (proc = 0; proc < nproc; proc++) {
> +             if (procs[proc].p_id != proc_id)
> +                     continue;
> +             p = &procs[proc];
> +             break;
> +     }
> +     if (p == NULL || p->p_init == NULL)
> +             fatalx("%s: process %d missing process initialization",
> +                 __func__, proc_id);
> +
> +     p->p_init(ps, p);
> +
> +     fatalx("failed to initiate child process");
> +}
> +
> +void
> +proc_accept(struct privsep *ps, int fd, enum privsep_procid dst,
> +    unsigned int n)
> +{
> +     struct privsep_pipes    *pp = ps->ps_pp;
> +     struct imsgev           *iev;
> +
> +     if (ps->ps_ievs[dst] == NULL) {
> +#if DEBUG > 1
> +             log_debug("%s: %s src %d %d to dst %d %d not connected",
> +                 __func__, ps->ps_title[privsep_process],
> +                 privsep_process, ps->ps_instance + 1,
> +                 dst, n + 1);
> +#endif

DPRINTF?

> +             close(fd);
> +             return;
> +     }
> +
> +     if (pp->pp_pipes[dst][n] != -1) {
> +             log_warnx("%s: duplicated descriptor", __func__);
> +             close(fd);
> +             return;
> +     } else
> +             pp->pp_pipes[dst][n] = fd;
> +
> +     iev = &ps->ps_ievs[dst][n];
> +     imsg_init(&iev->ibuf, fd);
> +     event_set(&iev->ev, iev->ibuf.fd, iev->events, iev->handler, iev->data);
> +     event_add(&iev->ev, NULL);
> +}
> +
>  void
> -proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
> +proc_setup(struct privsep *ps, struct privsep_proc *procs, unsigned int 
> nproc)
>  {
> -     unsigned int             i, j, src, dst;
> +     unsigned int             i, j, src, dst, id;
>       struct privsep_pipes    *pp;
>  
> +     /* Initialize parent title, ps_instances and procs. */
> +     ps->ps_title[PROC_PARENT] = "parent";
> +
>       for (src = 0; src < PROC_MAX; src++)
>               /* Default to 1 process instance */
>               if (ps->ps_instances[src] < 1)
>                       ps->ps_instances[src] = 1;
>  
> +     for (src = 0; src < nproc; src++) {
> +             procs[src].p_ps = ps;
> +             procs[src].p_env = ps->ps_env;
> +             if (procs[src].p_cb == NULL)
> +                     procs[src].p_cb = proc_dispatch_null;
> +
> +             id = procs[src].p_id;
> +             ps->ps_title[id] = procs[src].p_title;
> +             if ((ps->ps_ievs[id] = calloc(ps->ps_instances[id],
> +                 sizeof(struct imsgev))) == NULL)
> +                     fatal(__func__);
> +
> +             /* With this set up, we are ready to call imsg_init(). */
> +             for (i = 0; i < ps->ps_instances[id]; i++) {
> +                     ps->ps_ievs[id][i].handler = proc_dispatch;
> +                     ps->ps_ievs[id][i].events = EV_READ;
> +                     ps->ps_ievs[id][i].proc = &procs[src];
> +                     ps->ps_ievs[id][i].data = &ps->ps_ievs[id][i];
> +             }
> +     }
> +
>       /*
>        * Allocate pipes for all process instances (incl. parent)
>        *
> @@ -83,7 +336,7 @@ proc_init(struct privsep *ps, struct pri
>               /* Allocate destination array for each process */
>               if ((ps->ps_pipes[src] = calloc(ps->ps_instances[src],
>                   sizeof(struct privsep_pipes))) == NULL)
> -                     fatal("proc_init: calloc");
> +                     fatal("%s: calloc", __func__);
>  
>               for (i = 0; i < ps->ps_instances[src]; i++) {
>                       pp = &ps->ps_pipes[src][i];
> @@ -93,7 +346,7 @@ proc_init(struct privsep *ps, struct pri
>                               if ((pp->pp_pipes[dst] =
>                                   calloc(ps->ps_instances[dst],
>                                   sizeof(int))) == NULL)
> -                                     fatal("proc_init: calloc");
> +                                     fatal("%s: calloc", __func__);
>  
>                               /* Mark fd as unused */
>                               for (j = 0; j < ps->ps_instances[dst]; j++)
> @@ -102,22 +355,7 @@ proc_init(struct privsep *ps, struct pri
>               }
>       }
>  
> -     /*
> -      * Setup and run the parent and its children
> -      */
> -     privsep_process = PROC_PARENT;
> -     ps->ps_instances[PROC_PARENT] = 1;
> -     ps->ps_title[PROC_PARENT] = "parent";
> -     ps->ps_pp = &ps->ps_pipes[privsep_process][0];
> -
> -     for (i = 0; i < nproc; i++)
> -             ps->ps_title[procs[i].p_id] = procs[i].p_title;
> -
> -     proc_open(ps, NULL, procs, nproc);
> -
> -     /* Engage! */
> -     for (i = 0; i < nproc; i++)
> -             (*procs[i].p_init)(ps, &procs[i]);
> +     ps->ps_pp = &ps->ps_pipes[privsep_process][ps->ps_instance];
>  }
>  
>  void
> @@ -159,115 +397,30 @@ proc_kill(struct privsep *ps)
>  }
>  
>  void
> -proc_open(struct privsep *ps, struct privsep_proc *p,
> -    struct privsep_proc *procs, size_t nproc)
> +proc_open(struct privsep *ps, int src, int dst)
>  {
>       struct privsep_pipes    *pa, *pb;
>       int                      fds[2];
> -     unsigned int             i, j, src, proc;
> -
> -     if (p == NULL)
> -             src = privsep_process; /* parent */
> -     else
> -             src = p->p_id;
> -
> -     /*
> -      * Open socket pairs for our peers
> -      */
> -     for (proc = 0; proc < nproc; proc++) {
> -             procs[proc].p_ps = ps;
> -             procs[proc].p_env = ps->ps_env;
> -             if (procs[proc].p_cb == NULL)
> -                     procs[proc].p_cb = proc_dispatch_null;
> -
> -             for (i = 0; i < ps->ps_instances[src]; i++) {
> -                     for (j = 0; j < ps->ps_instances[procs[proc].p_id];
> -                         j++) {
> -                             pa = &ps->ps_pipes[src][i];
> -                             pb = &ps->ps_pipes[procs[proc].p_id][j];
> -
> -                             /* Check if fds are already set by peer */
> -                             if (pa->pp_pipes[procs[proc].p_id][j] != -1)
> -                                     continue;
> -
> -                             if (socketpair(AF_UNIX,
> -                                 SOCK_STREAM | SOCK_NONBLOCK,
> -                                 PF_UNSPEC, fds) == -1)
> -                                     fatal("socketpair");

maybe the logic below (/* Don't create sockets for ourself. */) can be used
here to not open so many fds' in the first place?

(not in this diff, just a suggestion for later)


> -                             pa->pp_pipes[procs[proc].p_id][j] = fds[0];
> -                             pb->pp_pipes[src][i] = fds[1];
> -                     }
> -             }
> -     }
> -}
> +     unsigned int             i, j;
>  
> -void
> -proc_listen(struct privsep *ps, struct privsep_proc *procs, size_t nproc)
> -{
> -     unsigned int             i, dst, src, n, m;
> -     struct privsep_pipes    *pp;
> -
> -     /*
> -      * Close unused pipes
> -      */
> -     for (src = 0; src < PROC_MAX; src++) {
> -             for (n = 0; n < ps->ps_instances[src]; n++) {
> -                     /* Ingore current process */
> -                     if (src == (unsigned int)privsep_process &&
> -                         n == ps->ps_instance)
> +     for (i = 0; i < ps->ps_instances[src]; i++) {
> +             for (j = 0; j < ps->ps_instances[dst]; j++) {
> +                     /* Don't create sockets for ourself. */
> +                     if (src == dst && i == j)
>                               continue;
>  
> -                     pp = &ps->ps_pipes[src][n];
> -
> -                     for (dst = 0; dst < PROC_MAX; dst++) {
> -                             if (src == dst)
> -                                     continue;
> -                             for (m = 0; m < ps->ps_instances[dst]; m++) {
> -                                     if (pp->pp_pipes[dst][m] == -1)
> -                                             continue;
> -
> -                                     /* Close and invalidate fd */
> -                                     close(pp->pp_pipes[dst][m]);
> -                                     pp->pp_pipes[dst][m] = -1;
> -                             }
> -                     }
> -             }
> -     }
> -
> -     src = privsep_process;
> -     ps->ps_pp = pp = &ps->ps_pipes[src][ps->ps_instance];
> -
> -     /*
> -      * Listen on appropriate pipes
> -      */
> -     for (i = 0; i < nproc; i++) {
> -             dst = procs[i].p_id;
> -
> -             if (src == dst)
> -                     fatal("proc_listen: cannot peer with oneself");
> -
> -             if ((ps->ps_ievs[dst] = calloc(ps->ps_instances[dst],
> -                 sizeof(struct imsgev))) == NULL)
> -                     fatal("proc_open");
> -
> -             for (n = 0; n < ps->ps_instances[dst]; n++) {
> -                     if (pp->pp_pipes[dst][n] == -1)
> +                     pa = &ps->ps_pipes[src][i];
> +                     pb = &ps->ps_pipes[dst][j];
> +                     if (pb->pp_pipes[dst][j] != -1)
>                               continue;
>  
> -                     imsg_init(&(ps->ps_ievs[dst][n].ibuf),
> -                         pp->pp_pipes[dst][n]);
> -                     ps->ps_ievs[dst][n].handler = proc_dispatch;
> -                     ps->ps_ievs[dst][n].events = EV_READ;
> -                     ps->ps_ievs[dst][n].proc = &procs[i];
> -                     ps->ps_ievs[dst][n].data = &ps->ps_ievs[dst][n];
> -
> -                     event_set(&(ps->ps_ievs[dst][n].ev),
> -                         ps->ps_ievs[dst][n].ibuf.fd,
> -                         ps->ps_ievs[dst][n].events,
> -                         ps->ps_ievs[dst][n].handler,
> -                         ps->ps_ievs[dst][n].data);
> -                     event_add(&(ps->ps_ievs[dst][n].ev), NULL);
> +                     if (socketpair(AF_UNIX,
> +                         SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
> +                         PF_UNSPEC, fds) == -1)
> +                             fatal(__func__);
> +
> +                     pa->pp_pipes[dst][j] = fds[0];
> +                     pb->pp_pipes[src][i] = fds[1];
>               }
>       }
>  }
> @@ -316,7 +469,7 @@ proc_shutdown(struct privsep_proc *p)
>  
>       log_info("%s exiting, pid %d", p->p_title, getpid());
>  
> -     _exit(0);
> +     exit(0);
>  }
>  
>  void
> @@ -346,32 +499,17 @@ proc_run(struct privsep *ps, struct priv
>      struct privsep_proc *procs, unsigned int nproc,
>      void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg)
>  {
> -     pid_t                    pid;
> -     struct passwd           *pw;
> +     struct passwd           *pw = ps->ps_pw;
>       const char              *root;
>       struct control_sock     *rcs;
> -     unsigned int             n;
>  
>       if (ps->ps_noaction)
> -             return;
> -
> -     proc_open(ps, p, procs, nproc);
> +             exit(0);
>  
> -     /* Fork child handlers */
> -     switch (pid = fork()) {
> -     case -1:
> -             fatal("proc_run: cannot fork");
> -     case 0:
> -             log_procinit(p->p_title);
> -
> -             /* Set the process group of the current process */
> -             setpgid(0, 0);
> -             break;
> -     default:
> -             return;
> -     }
> +     log_procinit(p->p_title);
>  
> -     pw = ps->ps_pw;
> +     /* 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)
> @@ -401,19 +539,6 @@ proc_run(struct privsep *ps, struct priv
>           setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
>               fatal("proc_run: cannot drop privileges");
>  
> -     /* Fork child handlers */
> -     for (n = 1; n < ps->ps_instances[p->p_id]; n++) {
> -             if (fork() == 0) {
> -                     ps->ps_instance = n;
> -                     break;
> -             }
> -     }
> -
> -#ifdef DEBUG
> -     log_debug("%s: %s %d/%d, pid %d", __func__, p->p_title,
> -         ps->ps_instance + 1, ps->ps_instances[p->p_id], getpid());
> -#endif

DPRINTF

> -
>       event_init();
>  
>       signal_set(&ps->ps_evsigint, SIGINT, proc_sig_handler, p);
> @@ -430,8 +555,8 @@ proc_run(struct privsep *ps, struct priv
>       signal_add(&ps->ps_evsigpipe, NULL);
>       signal_add(&ps->ps_evsigusr1, NULL);
>  
> -     proc_listen(ps, procs, nproc);
> -
> +     proc_setup(ps, procs, nproc);
> +     proc_accept(ps, PARENT_SOCK_FILENO, PROC_PARENT, 0);
>       if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) {
>               TAILQ_INIT(&ctl_conns);
>               if (control_listen(&ps->ps_csock) == -1)
> @@ -441,6 +566,11 @@ proc_run(struct privsep *ps, struct priv
>                               fatalx(__func__);
>       }
>  
> +#ifdef DEBUG
> +     log_debug("%s: %s %d/%d, pid %d", __func__, p->p_title,
> +         ps->ps_instance + 1, ps->ps_instances[p->p_id], getpid());
> +#endif
> +

DPRINTF

>       if (run != NULL)
>               run(ps, p, arg);
>  
> @@ -460,6 +590,7 @@ proc_dispatch(int fd, short event, void 
>       ssize_t                  n;
>       int                      verbose;
>       const char              *title;
> +     struct privsep_fd        pf;
>  
>       title = ps->ps_title[privsep_process];
>       ibuf = &iev->ibuf;
> @@ -509,6 +640,12 @@ proc_dispatch(int fd, short event, void 
>                       IMSG_SIZE_CHECK(&imsg, &verbose);
>                       memcpy(&verbose, imsg.data, sizeof(verbose));
>                       log_verbose(verbose);
> +                     break;
> +             case IMSG_CTL_PROCFD:
> +                     IMSG_SIZE_CHECK(&imsg, &pf);
> +                     memcpy(&pf, imsg.data, sizeof(pf));
> +                     proc_accept(ps, imsg.fd, pf.pf_procid,
> +                         pf.pf_instance);
>                       break;
>               default:
>                       log_warnx("%s: %s %d got invalid imsg %d peerid %d "
> 

-- 

Reply via email to