From 9326d639a3dfeb90c54cb9afa9501ba27c09edc4 Mon Sep 17 00:00:00 2001 From: reyk Date: Sun, 20 Apr 2014 14:48:29 +0000 Subject: [PATCH] Reimplement the multi-dimensional arrays that are used to set up the process to process imsg communication. It became a maze after we added support for multiple relay processes and even worse with the ca processes. This change makes it easier to understand. Now it only opens socketpairs that are needed - the code previously wasted lots of fds. ok blambert@ --- usr.sbin/relayd/proc.c | 283 +++++++++++++++++++++------------------ usr.sbin/relayd/relayd.c | 6 +- usr.sbin/relayd/relayd.h | 14 +- 3 files changed, 167 insertions(+), 136 deletions(-) diff --git a/usr.sbin/relayd/proc.c b/usr.sbin/relayd/proc.c index b8959ce211d..79d188b58ab 100644 --- a/usr.sbin/relayd/proc.c +++ b/usr.sbin/relayd/proc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: proc.c,v 1.10 2014/04/18 21:29:20 tedu Exp $ */ +/* $OpenBSD: proc.c,v 1.11 2014/04/20 14:48:29 reyk Exp $ */ /* * Copyright (c) 2010 - 2014 Reyk Floeter @@ -43,49 +43,88 @@ #include "relayd.h" -void proc_setup(struct privsep *); +void proc_open(struct privsep *, struct privsep_proc *, + struct privsep_proc *, size_t); +void proc_close(struct privsep *); int proc_ispeer(struct privsep_proc *, u_int, enum privsep_procid); void proc_shutdown(struct privsep_proc *); void proc_sig_handler(int, short, void *); void proc_range(struct privsep *, enum privsep_procid, int *, int *); -u_int proc_instances(struct privsep *, u_int, u_int); int -proc_ispeer(struct privsep_proc *p, u_int nproc, enum privsep_procid type) +proc_ispeer(struct privsep_proc *procs, u_int nproc, enum privsep_procid type) { u_int i; for (i = 0; i < nproc; i++) - if (p[i].p_id == type) + if (procs[i].p_id == type) return (1); return (0); } void -proc_init(struct privsep *ps, struct privsep_proc *p, u_int nproc) +proc_init(struct privsep *ps, struct privsep_proc *procs, u_int nproc) { - u_int i; + u_int i, j, src, dst; + struct privsep_pipes *pp; /* - * Called from parent + * Allocate pipes for all process instances (incl. parent) + * + * - ps->ps_pipes: N:M mapping + * N source processes connected to M destination processes: + * [src][instances][dst][instances], for example + * [PROC_RELAY][3][PROC_CA][3] + * + * - ps->ps_pp: per-process 1:M part of ps->ps_pipes + * Each process instance has a destination array of socketpair fds: + * [dst][instances], for example + * [PROC_PARENT][0] + */ + for (src = 0; src < PROC_MAX; src++) { + /* Allocate destination array for each process */ + if ((ps->ps_pipes[src] = calloc(ps->ps_ninstances, + sizeof(struct privsep_pipes))) == NULL) + fatal("proc_init: calloc"); + + for (i = 0; i < ps->ps_ninstances; i++) { + pp = &ps->ps_pipes[src][i]; + + for (dst = 0; dst < PROC_MAX; dst++) { + /* Allocate maximum fd integers */ + if ((pp->pp_pipes[dst] = + calloc(ps->ps_ninstances, + sizeof(int))) == NULL) + fatal("proc_init: calloc"); + + /* Mark fd as unused */ + for (j = 0; j < ps->ps_ninstances; j++) + pp->pp_pipes[dst][j] = -1; + } + } + } + + /* + * 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_pid[PROC_PARENT] = getpid(); + ps->ps_pp = &ps->ps_pipes[privsep_process][0]; for (i = 0; i < nproc; i++) { /* Default to 1 process instance */ - if (ps->ps_instances[p[i].p_id] < 1) - ps->ps_instances[p[i].p_id] = 1; - ps->ps_title[p[i].p_id] = p[i].p_title; + if (ps->ps_instances[procs[i].p_id] < 1) + ps->ps_instances[procs[i].p_id] = 1; + ps->ps_title[procs[i].p_id] = procs[i].p_title; } - proc_setup(ps); + proc_open(ps, NULL, procs, nproc); /* Engage! */ for (i = 0; i < nproc; i++) - ps->ps_pid[p[i].p_id] = (*p[i].p_init)(ps, &p[i]); + ps->ps_pid[procs[i].p_id] = (*procs[i].p_init)(ps, &procs[i]); } void @@ -107,101 +146,113 @@ proc_kill(struct privsep *ps) pid = waitpid(WAIT_ANY, NULL, 0); } while (pid != -1 || (pid == -1 && errno == EINTR)); - proc_clear(ps, 1); + proc_close(ps); } void -proc_setup(struct privsep *ps) +proc_open(struct privsep *ps, struct privsep_proc *p, + struct privsep_proc *procs, size_t nproc) { - int i, j, n, count, sockpair[2]; + struct privsep_pipes *pa, *pb; + int fds[2]; + u_int i, j, src, proc; - for (i = 0; i < PROC_MAX; i++) - for (j = 0; j < PROC_MAX; j++) { - if ((count = proc_instances(ps, i, j)) == 0) - continue; - - if ((ps->ps_pipes[i][j] = - calloc(count, sizeof(int))) == NULL || - (ps->ps_pipes[j][i] = - calloc(count, sizeof(int))) == NULL) - fatal(NULL); + if (p == NULL) + src = privsep_process; /* parent */ + else + src = p->p_id; - for (n = 0; n < count; n++) { - if (ps->ps_noaction) + /* + * Open socket pairs for our peers + */ + for (proc = 0; proc < nproc; proc++) { + procs[proc].p_ps = ps; + procs[proc].p_env = ps->ps_env; + + 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, PF_UNSPEC, - sockpair) == -1) + + if (socketpair(AF_UNIX, SOCK_STREAM, + PF_UNSPEC, fds) == -1) fatal("socketpair"); - ps->ps_pipes[i][j][n] = sockpair[0]; - ps->ps_pipes[j][i][n] = sockpair[1]; - socket_set_blockmode( - ps->ps_pipes[i][j][n], - BM_NONBLOCK); - socket_set_blockmode( - ps->ps_pipes[j][i][n], - BM_NONBLOCK); + + socket_set_blockmode(fds[0], BM_NONBLOCK); + socket_set_blockmode(fds[1], BM_NONBLOCK); + + pa->pp_pipes[procs[proc].p_id][j] = fds[0]; + pb->pp_pipes[src][i] = fds[1]; } } + } } void -proc_config(struct privsep *ps, struct privsep_proc *p, u_int nproc) +proc_listen(struct privsep *ps, struct privsep_proc *procs, size_t nproc) { - u_int i, j, src, dst, count, n, instance; - - src = privsep_process; + u_int i, dst, src, n, m; + struct privsep_pipes *pp; /* - * close unused pipes + * Close unused pipes */ - for (i = 0; i < PROC_MAX; i++) { - for (j = 0; j < PROC_MAX; j++) { - if ((count = proc_instances(ps, i, j)) == 0) + for (src = 0; src < PROC_MAX; src++) { + for (n = 0; n < ps->ps_instances[src]; n++) { + /* Ingore current process */ + if (src == (u_int)privsep_process && + n == ps->ps_instance) continue; - for (n = 0; n < count; n++) { - instance = ps->ps_instances[i] > 1 ? n : 0; - if (i == src && - proc_ispeer(p, nproc, j) && - ps->ps_instance == instance) - continue; + pp = &ps->ps_pipes[src][n]; - if (!ps->ps_noaction) - close(ps->ps_pipes[i][j][n]); - ps->ps_pipes[i][j][n] = -1; + 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 + * Listen on appropriate pipes */ for (i = 0; i < nproc; i++) { - dst = p[i].p_id; - p[i].p_ps = ps; - p[i].p_env = ps->ps_env; + dst = procs[i].p_id; if (src == dst) - fatal("proc_config: cannot peer with oneself"); + fatal("proc_listen: cannot peer with oneself"); - if ((count = proc_instances(ps, src, dst)) == 0) - continue; - - if ((ps->ps_ievs[dst] = calloc(count, + if ((ps->ps_ievs[dst] = calloc(ps->ps_instances[dst], sizeof(struct imsgev))) == NULL) - fatal("proc_config"); + fatal("proc_open"); - for (n = 0; n < count; n++) { - if (ps->ps_pipes[src][dst][n] == -1) + for (n = 0; n < ps->ps_instances[dst]; n++) { + if (pp->pp_pipes[dst][n] == -1) continue; imsg_init(&(ps->ps_ievs[dst][n].ibuf), - ps->ps_pipes[src][dst][n]); + 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 = &p[i]; + ps->ps_ievs[dst][n].proc = &procs[i]; ps->ps_ievs[dst][n].data = &ps->ps_ievs[dst][n]; - p[i].p_instance = n; + procs[i].p_instance = n; event_set(&(ps->ps_ievs[dst][n].ev), ps->ps_ievs[dst][n].ibuf.fd, @@ -214,53 +265,34 @@ proc_config(struct privsep *ps, struct privsep_proc *p, u_int nproc) } void -proc_clear(struct privsep *ps, int purge) +proc_close(struct privsep *ps) { - u_int src = privsep_process, dst, n, count; + u_int dst, n; + struct privsep_pipes *pp; if (ps == NULL) return; + pp = ps->ps_pp; + for (dst = 0; dst < PROC_MAX; dst++) { - if ((count = proc_instances(ps, src, dst)) == 0 || - ps->ps_ievs[dst] == NULL) + if (ps->ps_ievs[dst] == NULL) continue; - for (n = 0; n < count; n++) { - if (ps->ps_pipes[src][dst][n] == -1) + for (n = 0; n < ps->ps_instances[dst]; n++) { + if (pp->pp_pipes[dst][n] == -1) continue; - if (purge) { - event_del(&(ps->ps_ievs[dst][n].ev)); - imsg_clear(&(ps->ps_ievs[dst][n].ibuf)); - close(ps->ps_pipes[src][dst][n]); - } else - imsg_flush(&(ps->ps_ievs[dst][n].ibuf)); + + /* Cancel the fd, close and invalidate the fd */ + event_del(&(ps->ps_ievs[dst][n].ev)); + imsg_clear(&(ps->ps_ievs[dst][n].ibuf)); + close(pp->pp_pipes[dst][n]); + pp->pp_pipes[dst][n] = -1; } - if (purge) - free(ps->ps_ievs[dst]); + free(ps->ps_ievs[dst]); } } -u_int -proc_instances(struct privsep *ps, u_int src, u_int dst) -{ - u_int count; - - if (ps->ps_instances[src] > 1 && - ps->ps_instances[dst] > 1 && - ps->ps_instances[src] != ps->ps_instances[dst]) - fatalx("N:M peering not supported"); - - if (src == dst || - ps->ps_instances[src] == 0 || - ps->ps_instances[dst] == 0) - return (0); - - count = MAX(ps->ps_instances[src], ps->ps_instances[dst]); - - return (count); -} - void proc_shutdown(struct privsep_proc *p) { @@ -272,7 +304,7 @@ proc_shutdown(struct privsep_proc *p) if (p->p_shutdown != NULL) (*p->p_shutdown)(); - proc_clear(ps, 1); + proc_close(ps); log_info("%s exiting, pid %d", p->p_title, getpid()); @@ -313,9 +345,12 @@ proc_run(struct privsep *ps, struct privsep_proc *p, if (ps->ps_noaction) return (0); + proc_open(ps, p, procs, nproc); + + /* Fork child handlers */ switch (pid = fork()) { case -1: - fatal("run_proc: cannot fork"); + fatal("proc_run: cannot fork"); case 0: /* Set the process group of the current process */ setpgrp(0, getpid()); @@ -326,7 +361,7 @@ proc_run(struct privsep *ps, struct privsep_proc *p, pw = ps->ps_pw; - if (p->p_id == PROC_CONTROL) { + if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) { if (control_init(ps, &ps->ps_csock) == -1) fatalx(p->p_title); } @@ -339,16 +374,16 @@ proc_run(struct privsep *ps, struct privsep_proc *p, #ifndef DEBUG if (chroot(root) == -1) - fatal("run_proc: chroot"); + fatal("proc_run: chroot"); if (chdir("/") == -1) - fatal("run_proc: chdir(\"/\")"); + fatal("proc_run: chdir(\"/\")"); #else #warning disabling privilege revocation and chroot in DEBUG MODE if (p->p_chroot != NULL) { if (chroot(root) == -1) - fatal("run_proc: chroot"); + fatal("proc_run: chroot"); if (chdir("/") == -1) - fatal("run_proc: chdir(\"/\")"); + fatal("proc_run: chdir(\"/\")"); } #endif @@ -360,7 +395,7 @@ proc_run(struct privsep *ps, struct privsep_proc *p, if (setgroups(1, &pw->pw_gid) || setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) || setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid)) - fatal("run_proc: cannot drop privileges"); + fatal("proc_run: cannot drop privileges"); #endif /* Fork child handlers */ @@ -390,7 +425,7 @@ proc_run(struct privsep *ps, struct privsep_proc *p, signal_add(&ps->ps_evsighup, NULL); signal_add(&ps->ps_evsigpipe, NULL); - proc_config(ps, procs, nproc); + proc_listen(ps, procs, nproc); if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) { TAILQ_INIT(&ctl_conns); @@ -531,22 +566,9 @@ void proc_range(struct privsep *ps, enum privsep_procid id, int *n, int *m) { if (*n == -1) { - /* - * -1 means that the current process is - * N:1 - one of many processes connected to a single peer, - * so find the right slot of the peer. - * 1:N - a single process connected to many peers, - * so find the range of slots of all peers. - */ - if (ps->ps_instances[privsep_process] <= - ps->ps_instances[id]) { - *n = 0; - *m = ps->ps_instances[id]; - return; - } - - *n = ps->ps_instance; - *m = ps->ps_instance + 1; + /* Use a range of all target instances */ + *n = 0; + *m = ps->ps_instances[id]; } else { /* Use only a single slot of the specified peer process */ *m = *n + 1; @@ -560,10 +582,11 @@ proc_compose_imsg(struct privsep *ps, enum privsep_procid id, int n, int m; proc_range(ps, id, &n, &m); - for (; n < m; n++) + for (; n < m; n++) { if (imsg_compose_event(&ps->ps_ievs[id][n], type, -1, 0, fd, data, datalen) == -1) return (-1); + } return (0); } diff --git a/usr.sbin/relayd/relayd.c b/usr.sbin/relayd/relayd.c index 6051bd55278..afabd002224 100644 --- a/usr.sbin/relayd/relayd.c +++ b/usr.sbin/relayd/relayd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: relayd.c,v 1.120 2014/04/18 13:55:26 reyk Exp $ */ +/* $OpenBSD: relayd.c,v 1.121 2014/04/20 14:48:29 reyk Exp $ */ /* * Copyright (c) 2007 - 2014 Reyk Floeter @@ -226,6 +226,8 @@ main(int argc, char *argv[]) ps->ps_instances[PROC_RELAY] = env->sc_prefork_relay; ps->ps_instances[PROC_CA] = env->sc_prefork_relay; + ps->ps_ninstances = env->sc_prefork_relay; + proc_init(ps, procs, nitems(procs)); setproctitle("parent"); @@ -244,7 +246,7 @@ main(int argc, char *argv[]) signal_add(&ps->ps_evsighup, NULL); signal_add(&ps->ps_evsigpipe, NULL); - proc_config(ps, procs, nitems(procs)); + proc_listen(ps, procs, nitems(procs)); if (load_config(env->sc_conffile, env) == -1) { proc_kill(env->sc_ps); diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h index bce53dff249..559b167deab 100644 --- a/usr.sbin/relayd/relayd.h +++ b/usr.sbin/relayd/relayd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: relayd.h,v 1.175 2014/04/18 13:55:26 reyk Exp $ */ +/* $OpenBSD: relayd.h,v 1.176 2014/04/20 14:48:29 reyk Exp $ */ /* * Copyright (c) 2006 - 2014 Reyk Floeter @@ -860,14 +860,21 @@ enum privsep_procid { /* Attach the control socket to the following process */ #define PROC_CONTROL PROC_PFE +struct privsep_pipes { + int *pp_pipes[PROC_MAX]; +}; + struct privsep { - int *ps_pipes[PROC_MAX][PROC_MAX]; + struct privsep_pipes *ps_pipes[PROC_MAX]; + struct privsep_pipes *ps_pp; + struct imsgev *ps_ievs[PROC_MAX]; const char *ps_title[PROC_MAX]; pid_t ps_pid[PROC_MAX]; u_int8_t ps_what[PROC_MAX]; u_int ps_instances[PROC_MAX]; + u_int ps_ninstances; u_int ps_instance; struct control_sock ps_csock; @@ -1178,8 +1185,7 @@ __dead void fatalx(const char *); /* proc.c */ void proc_init(struct privsep *, struct privsep_proc *, u_int); void proc_kill(struct privsep *); -void proc_clear(struct privsep *, int); -void proc_config(struct privsep *, struct privsep_proc *, u_int); +void proc_listen(struct privsep *, struct privsep_proc *, size_t); void proc_dispatch(int, short event, void *); pid_t proc_run(struct privsep *, struct privsep_proc *, struct privsep_proc *, u_int, -- 2.20.1