On Sat, Sep 03, 2016 at 11:42:54AM +0200, Sebastian Benoit wrote:
> ok, comments below
>
The proc.c is identical (except one #include) to httpd, I'll update
the DPRINTFs in httpd's proc.c accordingly and sync it with relayd
later. In one case it is DEBUG > 1, so no DPRINTF is needed.
Reyk
> Reyk Floeter(r...@openbsd.org) 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 <r...@openbsd.org>
> > @@ -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 "
> >
>
> --
--
