Rafael Zalamena <[email protected]> writes:

> On Fri, Oct 14, 2016 at 06:47:09PM +0200, Rafael Zalamena wrote:
>> On Mon, Sep 26, 2016 at 03:45:59PM +0200, Rafael Zalamena wrote:
>> > Lets teach snmpd(8) how to fork+exec using the proc.c file from the latest
>> > switchd(8) diff.
>> > 
>> > Note 1: I just tested the basic operations: startup and teardown.
>> > Note 2: the kill with close will be implemented in another diff with the
>> >         ps_pid removal.
>> > 
>> 
>> I've update the diff with the latest proc.c changes.
>> 
>> Note: I still have to implement kill with close().
>> 
>
> I got feedback from jca@ that the trap handler wasn't working, so after
> trying to reproduce the problem myself I found one 'env' global variable
> that was not being set and the child process was dying silently.
> (thanks jca@ !)
>
> Instead of depending on snmpe.c:snmpe env initialization (p_init), I'm
> now calling smi_setenv() to do that in the main() function so all children
> get the same behaviour. Also note that we don't have an 'extern' env in
> smi.c anymore.
>
> ok?

Works fine here, but then I don't understand the relationship between
static struct snmpd *env in smi.c and struct snmpd *env in snmpe.c.

> Index: proc.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/proc.c,v
> retrieving revision 1.20
> diff -u -p -r1.20 proc.c
> --- proc.c    7 Dec 2015 16:05:56 -0000       1.20
> +++ proc.c    14 Oct 2016 15:42:19 -0000
> @@ -1,7 +1,7 @@
>  /*   $OpenBSD: proc.c,v 1.20 2015/12/07 16:05:56 reyk Exp $  */
>  
>  /*
> - * Copyright (c) 2010 - 2014 Reyk Floeter <[email protected]>
> + * Copyright (c) 2010 - 2016 Reyk Floeter <[email protected]>
>   * Copyright (c) 2008 Pierre-Yves Ritschard <[email protected]>
>   *
>   * Permission to use, copy, modify, and distribute this software for any
> @@ -22,6 +22,7 @@
>  #include <sys/socket.h>
>  #include <sys/wait.h>
>  
> +#include <fcntl.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> @@ -34,8 +35,12 @@
>  
>  #include "snmpd.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_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,204 +60,383 @@ proc_ispeer(struct privsep_proc *procs, 
>       return (0);
>  }
>  
> -void
> -proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
> +enum privsep_procid
> +proc_getid(struct privsep_proc *procs, unsigned int nproc,
> +    const char *proc_name)
>  {
> -     unsigned int             i, j, src, dst;
> -     struct privsep_pipes    *pp;
> +     struct privsep_proc     *p;
> +     unsigned int             proc;
>  
> -     /*
> -      * 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 (proc = 0; proc < nproc; proc++) {
> +             p = &procs[proc];
> +             if (strcmp(p->p_title, proc_name))
> +                     continue;
>  
> -             for (i = 0; i < ps->ps_ninstances; i++) {
> -                     pp = &ps->ps_pipes[src][i];
> +             return (p->p_id);
> +     }
>  
> -                     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");
> +     return (PROC_MAX);
> +}
>  
> -                             /* Mark fd as unused */
> -                             for (j = 0; j < ps->ps_ninstances; j++)
> -                                     pp->pp_pipes[dst][j] = -1;
> -                     }
> -             }
> -     }
> +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];
>  
> -     /*
> -      * 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];
> +     nargv[nargc] = NULL;
>  
> -     for (i = 0; i < nproc; i++) {
> -             /* Default to 1 process instance */
> -             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;
> -     }
> +     for (proc = 0; proc < nproc; proc++) {
> +             p = &procs[proc];
>  
> -     proc_open(ps, NULL, procs, nproc);
> +             /* Update args with process title. */
> +             nargv[proc_i] = (char *)(uintptr_t)p->p_title;
>  
> -     /* Engage! */
> -     for (i = 0; i < nproc; i++)
> -             ps->ps_pid[procs[i].p_id] = (*procs[i].p_init)(ps, &procs[i]);
> +             /* 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:
> +                             /* 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)
> +                                         == -1)
> +                                             fatal("dup2");
> +                             } else if (fcntl(fd, F_SETFD, 0) == -1)
> +                                     fatal("fcntl");
> +
> +                             execvp(argv[0], nargv);
> +                             fatal("%s: execvp", __func__);
> +                             break;
> +                     default:
> +                             /* Close child end. */
> +                             close(fd);
> +                             break;
> +                     }
> +             }
> +     }
> +     free(nargv);
>  }
>  
>  void
> -proc_kill(struct privsep *ps)
> +proc_connect(struct privsep *ps)
>  {
> -     pid_t            pid;
> -     unsigned int     i;
> +     struct imsgev           *iev;
> +     unsigned int             src, dst, inst;
>  
> -     if (privsep_process != PROC_PARENT)
> +     /* Don't distribute any sockets if we are not really going to run. */
> +     if (ps->ps_noaction)
>               return;
>  
> -     for (i = 0; i < PROC_MAX; i++) {
> -             if (ps->ps_pid[i] == 0)
> +     for (dst = 0; dst < PROC_MAX; dst++) {
> +             /* We don't communicate with ourselves. */
> +             if (dst == PROC_PARENT)
>                       continue;
> -             killpg(ps->ps_pid[i], SIGTERM);
> +
> +             for (inst = 0; inst < ps->ps_instances[dst]; inst++) {
> +                     iev = &ps->ps_ievs[dst][inst];
> +                     imsg_init(&iev->ibuf, ps->ps_pp->pp_pipes[dst][inst]);
> +                     event_set(&iev->ev, iev->ibuf.fd, iev->events,
> +                         iev->handler, iev->data);
> +                     event_add(&iev->ev, NULL);
> +             }
>       }
>  
> -     do {
> -             pid = waitpid(WAIT_ANY, NULL, 0);
> -     } while (pid != -1 || (pid == -1 && errno == EINTR));
> +     /* Distribute the socketpair()s for everyone. */
> +     for (src = 0; src < PROC_MAX; src++)
> +             for (dst = src; dst < PROC_MAX; dst++) {
> +                     /* Parent already distributed its fds. */
> +                     if (src == PROC_PARENT || dst == PROC_PARENT)
> +                             continue;
>  
> -     proc_close(ps);
> +                     proc_open(ps, src, dst);
> +             }
>  }
>  
>  void
> -proc_open(struct privsep *ps, struct privsep_proc *p,
> -    struct privsep_proc *procs, size_t nproc)
> +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;
>       struct privsep_pipes    *pa, *pb;
> +     unsigned int             proc;
> +     unsigned int             dst;
>       int                      fds[2];
> -     unsigned int             i, j, src, proc;
>  
> -     if (p == NULL)
> -             src = privsep_process; /* parent */
> -     else
> -             src = p->p_id;
> +     /* Don't initiate anything if we are not really going to run. */
> +     if (ps->ps_noaction)
> +             return;
>  
> -     /*
> -      * 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;
> +     if (proc_id == PROC_PARENT) {
> +             privsep_process = PROC_PARENT;
> +             proc_setup(ps, procs, nproc);
>  
> -             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;
> +             /*
> +              * Create the children sockets so we can use them 
> +              * to distribute the rest of the socketpair()s using
> +              * proc_connect() later.
> +              */
> +             for (dst = 0; dst < PROC_MAX; dst++) {
> +                     /* Don't create socket for ourselves. */
> +                     if (dst == PROC_PARENT)
> +                             continue;
>  
> +                     for (proc = 0; proc < ps->ps_instances[dst]; proc++) {
> +                             pa = &ps->ps_pipes[PROC_PARENT][0];
> +                             pb = &ps->ps_pipes[dst][proc];
>                               if (socketpair(AF_UNIX,
> -                                 SOCK_STREAM | SOCK_NONBLOCK,
> +                                 SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
>                                   PF_UNSPEC, fds) == -1)
> -                                     fatal("socketpair");
> +                                     fatal("%s: socketpair", __func__);
>  
> -                             pa->pp_pipes[procs[proc].p_id][j] = fds[0];
> -                             pb->pp_pipes[src][i] = fds[1];
> +                             pa->pp_pipes[dst][proc] = fds[0];
> +                             pb->pp_pipes[PROC_PARENT][0] = fds[1];
>                       }
>               }
> +
> +             /* 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_listen(struct privsep *ps, struct privsep_proc *procs, size_t nproc)
> +proc_accept(struct privsep *ps, int fd, enum privsep_procid dst,
> +    unsigned int n)
>  {
> -     unsigned int             i, dst, src, n, m;
> +     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
> +             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_setup(struct privsep *ps, struct privsep_proc *procs, unsigned int 
> nproc)
> +{
> +     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;
> +             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("%s: calloc", __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];
> +             }
> +     }
> +
>       /*
> -      * Close unused pipes
> +      * 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++) {
> -             for (n = 0; n < ps->ps_instances[src]; n++) {
> -                     /* Ingore current process */
> -                     if (src == (unsigned int)privsep_process &&
> -                         n == ps->ps_instance)
> -                             continue;
> +             /* Allocate destination array for each process */
> +             if ((ps->ps_pipes[src] = calloc(ps->ps_instances[src],
> +                 sizeof(struct privsep_pipes))) == NULL)
> +                     fatal("%s: calloc", __func__);
>  
> -                     pp = &ps->ps_pipes[src][n];
> +             for (i = 0; i < ps->ps_instances[src]; i++) {
> +                     pp = &ps->ps_pipes[src][i];
>  
>                       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;
> -                             }
> +                             /* Allocate maximum fd integers */
> +                             if ((pp->pp_pipes[dst] =
> +                                 calloc(ps->ps_instances[dst],
> +                                 sizeof(int))) == NULL)
> +                                     fatal("%s: calloc", __func__);
> +
> +                             /* Mark fd as unused */
> +                             for (j = 0; j < ps->ps_instances[dst]; j++)
> +                                     pp->pp_pipes[dst][j] = -1;
>                       }
>               }
>       }
>  
> -     src = privsep_process;
> -     ps->ps_pp = pp = &ps->ps_pipes[src][ps->ps_instance];
> +     ps->ps_pp = &ps->ps_pipes[privsep_process][ps->ps_instance];
> +}
>  
> -     /*
> -      * Listen on appropriate pipes
> -      */
> -     for (i = 0; i < nproc; i++) {
> -             dst = procs[i].p_id;
> +void
> +proc_kill(struct privsep *ps)
> +{
> +     char            *cause;
> +     pid_t            pid;
> +     int              len, status;
>  
> -             if (src == dst)
> -                     fatal("proc_listen: cannot peer with oneself");
> +     if (privsep_process != PROC_PARENT)
> +             return;
>  
> -             if ((ps->ps_ievs[dst] = calloc(ps->ps_instances[dst],
> -                 sizeof(struct imsgev))) == NULL)
> -                     fatal("proc_open");
> +     proc_close(ps);
>  
> -             for (n = 0; n < ps->ps_instances[dst]; n++) {
> -                     if (pp->pp_pipes[dst][n] == -1)
> +     do {
> +             pid = waitpid(WAIT_ANY, &status, 0);
> +             if (pid <= 0)
> +                     continue;
> +
> +             if (WIFSIGNALED(status)) {
> +                     len = asprintf(&cause, "terminated; signal %d",
> +                         WTERMSIG(status));
> +             } else if (WIFEXITED(status)) {
> +                     if (WEXITSTATUS(status) != 0)
> +                             len = asprintf(&cause, "exited abnormally");
> +                     else
> +                             len = 0;
> +             } else
> +                     len = -1;
> +
> +             if (len == 0) {
> +                     /* child exited OK, don't print a warning message */
> +             } else if (len != -1) {
> +                     log_warnx("lost child: pid %u %s", pid, cause);
> +                     free(cause);
> +             } else
> +                     log_warnx("lost child: pid %u", pid);
> +     } while (pid != -1 || (pid == -1 && errno == EINTR));
> +}
> +
> +void
> +proc_open(struct privsep *ps, int src, int dst)
> +{
> +     struct privsep_pipes    *pa, *pb;
> +     struct privsep_fd        pf;
> +     int                      fds[2];
> +     unsigned int             i, j;
> +
> +     /* Exchange pipes between process. */
> +     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;
>  
> -                     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];
> -                     procs[i].p_instance = 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);
> +                     pa = &ps->ps_pipes[src][i];
> +                     pb = &ps->ps_pipes[dst][j];
> +                     if (socketpair(AF_UNIX,
> +                         SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
> +                         PF_UNSPEC, fds) == -1)
> +                             fatal("%s: socketpair", __func__);
> +
> +                     pa->pp_pipes[dst][j] = fds[0];
> +                     pb->pp_pipes[src][i] = fds[1];
> +
> +                     pf.pf_procid = src;
> +                     pf.pf_instance = i;
> +                     if (proc_compose_imsg(ps, dst, j, IMSG_CTL_PROCFD,
> +                         -1, pb->pp_pipes[src][i], &pf, sizeof(pf)) == -1)
> +                             fatal("%s: proc_compose_imsg", __func__);
> +
> +                     pf.pf_procid = dst;
> +                     pf.pf_instance = j;
> +                     if (proc_compose_imsg(ps, src, i, IMSG_CTL_PROCFD,
> +                         -1, pa->pp_pipes[dst][j], &pf, sizeof(pf)) == -1)
> +                             fatal("%s: proc_compose_imsg", __func__);
> +
> +                     /*
> +                      * We have to flush to send the descriptors and close
> +                      * them to avoid the fd ramp on startup.
> +                      */
> +                     if (proc_flush_imsg(ps, src, i) == -1 ||
> +                         proc_flush_imsg(ps, dst, j) == -1)
> +                             fatal("%s: imsg_flush", __func__);
>               }
>       }
>  }
> @@ -301,7 +485,7 @@ proc_shutdown(struct privsep_proc *p)
>  
>       log_info("%s exiting, pid %d", p->p_title, getpid());
>  
> -     _exit(0);
> +     exit(0);
>  }
>  
>  void
> @@ -326,46 +510,34 @@ proc_sig_handler(int sig, short event, v
>       }
>  }
>  
> -pid_t
> +void
>  proc_run(struct privsep *ps, struct privsep_proc *p,
>      struct privsep_proc *procs, unsigned int nproc,
>      void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg)
>  {
> -     pid_t                    pid;
>       struct passwd           *pw;
>       const char              *root;
>       struct control_sock     *rcs;
> -     unsigned int             n;
>  
> -     if (ps->ps_noaction)
> -             return (0);
> -
> -     proc_open(ps, p, procs, nproc);
> -
> -     /* Fork child handlers */
> -     switch (pid = fork()) {
> -     case -1:
> -             fatal("proc_run: cannot fork");
> -     case 0:
> -             log_procinit(p->p_title);
> +     log_procinit(p->p_title);
>  
> -             /* Set the process group of the current process */
> -             setpgid(0, 0);
> -             break;
> -     default:
> -             return (pid);
> -     }
> -
> -     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)
> -                     fatalx(__func__);
> +                     fatalx("%s: control_init", __func__);
>               TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
>                       if (control_init(ps, rcs) == -1)
> -                             fatalx(__func__);
> +                             fatalx("%s: control_init", __func__);
>       }
>  
> +     /* Use non-standard user */
> +     if (p->p_pw != NULL)
> +             pw = p->p_pw;
> +     else
> +             pw = ps->ps_pw;
> +
>       /* Change root directory */
>       if (p->p_chroot != NULL)
>               root = p->p_chroot;
> @@ -386,19 +558,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 = p->p_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
> -
>       event_init();
>  
>       signal_set(&ps->ps_evsigint, SIGINT, proc_sig_handler, p);
> @@ -415,25 +574,26 @@ 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, PROC_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)
> -                     fatalx(__func__);
> +                     fatalx("%s: control_listen", __func__);
>               TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
>                       if (control_listen(rcs) == -1)
> -                             fatalx(__func__);
> +                             fatalx("%s: control_listen", __func__);
>       }
>  
> +     DPRINTF("%s: %s %d/%d, pid %d", __func__, p->p_title,
> +         ps->ps_instance + 1, ps->ps_instances[p->p_id], getpid());
> +
>       if (run != NULL)
>               run(ps, p, arg);
>  
>       event_dispatch();
>  
>       proc_shutdown(p);
> -
> -     return (0);
>  }
>  
>  void
> @@ -447,13 +607,14 @@ 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;
>  
>       if (event & EV_READ) {
>               if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
> -                     fatal(__func__);
> +                     fatal("%s: imsg_read", __func__);
>               if (n == 0) {
>                       /* this pipe is dead, so remove the event handler */
>                       event_del(&iev->ev);
> @@ -463,20 +624,26 @@ proc_dispatch(int fd, short event, void 
>       }
>  
>       if (event & EV_WRITE) {
> -             if (msgbuf_write(&ibuf->w) <= 0 && errno != EAGAIN)
> -                     fatal(__func__);
> +             if ((n = msgbuf_write(&ibuf->w)) == -1 && errno != EAGAIN)
> +                     fatal("%s: msgbuf_write", __func__);
> +             if (n == 0) {
> +                     /* this pipe is dead, so remove the event handler */
> +                     event_del(&iev->ev);
> +                     event_loopexit(NULL);
> +                     return;
> +             }
>       }
>  
>       for (;;) {
>               if ((n = imsg_get(ibuf, &imsg)) == -1)
> -                     fatal(__func__);
> +                     fatal("%s: imsg_get", __func__);
>               if (n == 0)
>                       break;
>  
>  #if DEBUG > 1
>               log_debug("%s: %s %d got imsg %d peerid %d from %s %d",
>                   __func__, title, ps->ps_instance + 1,
> -                 imsg.hdr.type, imsg.hdr.peerid, p->p_title, p->p_instance);
> +                 imsg.hdr.type, imsg.hdr.peerid, p->p_title, imsg.hdr.pid);
>  #endif
>  
>               /*
> @@ -497,13 +664,18 @@ proc_dispatch(int fd, short event, void 
>                       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 "
> +                     fatalx("%s: %s %d got invalid imsg %d peerid %d "
>                           "from %s %d",
>                           __func__, title, ps->ps_instance + 1,
>                           imsg.hdr.type, imsg.hdr.peerid,
> -                         p->p_title, p->p_instance);
> -                     fatalx(__func__);
> +                         p->p_title, imsg.hdr.pid);
>               }
>               imsg_free(&imsg);
>       }
> @@ -585,7 +757,7 @@ proc_compose_imsg(struct privsep *ps, en
>       proc_range(ps, id, &n, &m);
>       for (; n < m; n++) {
>               if (imsg_compose_event(&ps->ps_ievs[id][n],
> -                 type, peerid, 0, fd, data, datalen) == -1)
> +                 type, peerid, ps->ps_instance + 1, fd, data, datalen) == -1)
>                       return (-1);
>       }
>  
> @@ -608,7 +780,7 @@ proc_composev_imsg(struct privsep *ps, e
>       proc_range(ps, id, &n, &m);
>       for (; n < m; n++)
>               if (imsg_composev_event(&ps->ps_ievs[id][n],
> -                 type, peerid, 0, fd, iov, iovcnt) == -1)
> +                 type, peerid, ps->ps_instance + 1, fd, iov, iovcnt) == -1)
>                       return (-1);
>  
>       return (0);
> @@ -645,4 +817,26 @@ proc_iev(struct privsep *ps, enum privse
>  
>       proc_range(ps, id, &n, &m);
>       return (&ps->ps_ievs[id][n]);
> +}
> +
> +/* This function should only be called with care as it breaks async I/O */
> +int
> +proc_flush_imsg(struct privsep *ps, enum privsep_procid id, int n)
> +{
> +     struct imsgbuf  *ibuf;
> +     int              m, ret = 0;
> +
> +     proc_range(ps, id, &n, &m);
> +     for (; n < m; n++) {
> +             if ((ibuf = proc_ibuf(ps, id, n)) == NULL)
> +                     return (-1);
> +             do {
> +                     ret = imsg_flush(ibuf);
> +             } while (ret == -1 && errno == EAGAIN);
> +             if (ret == -1)
> +                     break;
> +             imsg_event_add(&ps->ps_ievs[id][n]);
> +     }
> +
> +     return (ret);
>  }
> Index: smi.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/smi.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 smi.c
> --- smi.c     5 Dec 2015 06:42:18 -0000       1.19
> +++ smi.c     21 Oct 2016 10:55:26 -0000
> @@ -49,7 +49,7 @@
>  
>  #define MINIMUM(a, b)        (((a) < (b)) ? (a) : (b))
>  
> -extern struct snmpd *env;
> +static struct snmpd *env;
>  
>  RB_HEAD(oidtree, oid);
>  RB_PROTOTYPE(oidtree, oid, o_element, smi_oid_cmp);
> @@ -235,6 +235,12 @@ smi_init(void)
>       RB_INIT(&smi_oidtree);
>       mib_init();
>       return (0);
> +}
> +
> +void
> +smi_setenv(struct snmpd *xenv)
> +{
> +     env = xenv;
>  }
>  
>  struct oid *
> Index: snmpd.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/snmpd.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 snmpd.c
> --- snmpd.c   16 Aug 2016 18:41:57 -0000      1.33
> +++ snmpd.c   21 Oct 2016 10:54:03 -0000
> @@ -144,13 +144,17 @@ main(int argc, char *argv[])
>       int              noaction = 0;
>       const char      *conffile = CONF_FILE;
>       struct privsep  *ps;
> +     int              proc_id = PROC_PARENT, proc_instance = 0;
> +     int              argc0 = argc;
> +     char            **argv0 = argv;
> +     const char      *errp, *title = NULL;
>  
>       smi_init();
>  
>       /* log to stderr until daemonized */
>       log_init(1, LOG_DAEMON);
>  
> -     while ((c = getopt(argc, argv, "dD:nNf:v")) != -1) {
> +     while ((c = getopt(argc, argv, "dD:nNf:I:P:v")) != -1) {
>               switch (c) {
>               case 'd':
>                       debug++;
> @@ -169,6 +173,18 @@ main(int argc, char *argv[])
>               case 'f':
>                       conffile = optarg;
>                       break;
> +             case 'I':
> +                     proc_instance = strtonum(optarg, 0, PROC_MAX_INSTANCES,
> +                         &errp);
> +                     if (errp)
> +                             fatalx("invalid process instance");
> +                     break;
> +             case 'P':
> +                     title = optarg;
> +                     proc_id = proc_getid(procs, nitems(procs), title);
> +                     if (proc_id == PROC_MAX)
> +                             fatalx("invalid process name");
> +                     break;
>               case 'v':
>                       verbose++;
>                       flags |= SNMPD_F_VERBOSE;
> @@ -186,9 +202,13 @@ main(int argc, char *argv[])
>       if ((env = parse_config(conffile, flags)) == NULL)
>               exit(1);
>  
> +     smi_setenv(env);
>       ps = &env->sc_ps;
>       ps->ps_env = env;
>       snmpd_env = env;
> +     ps->ps_instance = proc_instance;
> +     if (title)
> +             ps->ps_title[proc_id] = title;
>  
>       if (noaction) {
>               fprintf(stderr, "configuration ok\n");
> @@ -204,17 +224,15 @@ main(int argc, char *argv[])
>       log_init(debug, LOG_DAEMON);
>       log_verbose(verbose);
>  
> -     if (!debug && daemon(0, 0) == -1)
> -             err(1, "failed to daemonize");
> -
>       gettimeofday(&env->sc_starttime, NULL);
>       env->sc_engine_boots = 0;
>  
>       pf_init();
>       snmpd_generate_engineid(env);
>  
> -     ps->ps_ninstances = 1;
> -     proc_init(ps, procs, nitems(procs));
> +     proc_init(ps, procs, nitems(procs), argc0, argv0, proc_id);
> +     if (!debug && daemon(0, 0) == -1)
> +             err(1, "failed to daemonize");
>  
>       log_procinit("parent");
>       log_info("startup");
> @@ -235,7 +253,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);
>  
>       event_dispatch();
>  
> Index: snmpd.h
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/snmpd.h,v
> retrieving revision 1.69
> diff -u -p -r1.69 snmpd.h
> --- snmpd.h   3 Oct 2016 12:16:41 -0000       1.69
> +++ snmpd.h   21 Oct 2016 10:53:46 -0000
> @@ -87,6 +87,7 @@ enum imsg_type {
>       IMSG_CTL_NOTIFY,
>       IMSG_CTL_VERBOSE,
>       IMSG_CTL_RELOAD,
> +     IMSG_CTL_PROCFD,
>       IMSG_ALERT
>  };
>  
> @@ -146,7 +147,6 @@ struct privsep {
>       struct passwd           *ps_pw;
>  
>       u_int                    ps_instances[PROC_MAX];
> -     u_int                    ps_ninstances;
>       u_int                    ps_instance;
>       int                      ps_noaction;
>  
> @@ -169,15 +169,28 @@ struct privsep_proc {
>       enum privsep_procid      p_id;
>       int                     (*p_cb)(int, struct privsep_proc *,
>                                   struct imsg *);
> -     pid_t                   (*p_init)(struct privsep *,
> +     void                    (*p_init)(struct privsep *,
>                                   struct privsep_proc *);
>       void                    (*p_shutdown)(void);
>       const char              *p_chroot;
>       struct privsep          *p_ps;
> -     void                    *p_env;
> -     u_int                    p_instance;
> +     struct passwd           *p_pw;
>  };
>  
> +struct privsep_fd {
> +     enum privsep_procid              pf_procid;
> +     unsigned int                     pf_instance;
> +};
> +
> +#define PROC_PARENT_SOCK_FILENO      3
> +#define PROC_MAX_INSTANCES   32
> +
> +#if DEBUG
> +#define DPRINTF              log_debug
> +#else
> +#define DPRINTF(x...)        do {} while(0)
> +#endif
> +
>  /*
>   * kroute
>   */
> @@ -631,7 +644,7 @@ struct kif_arp    *karp_first(u_short);
>  struct kif_arp       *karp_getaddr(struct sockaddr *, u_short, int);
>  
>  /* snmpe.c */
> -pid_t                 snmpe(struct privsep *, struct privsep_proc *);
> +void          snmpe(struct privsep *, struct privsep_proc *);
>  void          snmpe_shutdown(void);
>  void          snmpe_dispatchmsg(struct snmp_message *);
>  
> @@ -689,6 +702,7 @@ int                        pfta_get_first(struct 
> pfr_astats 
>  
>  /* smi.c */
>  int           smi_init(void);
> +void          smi_setenv(struct snmpd *);
>  u_long                smi_getticks(void);
>  void          smi_mibtree(struct oid *);
>  struct oid   *smi_find(struct oid *);
> @@ -728,11 +742,14 @@ void             usm_finalize_digest(struct snmp_m
>  void          usm_make_report(struct snmp_message *);
>  
>  /* 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 *);
> -pid_t         proc_run(struct privsep *, struct privsep_proc *,
> +void  proc_run(struct privsep *, struct privsep_proc *,
>           struct privsep_proc *, u_int,
>           void (*)(struct privsep *, struct privsep_proc *, void *), void *);
>  void  imsg_event_add(struct imsgev *);
> @@ -755,9 +772,10 @@ struct imsgbuf *
>        proc_ibuf(struct privsep *, enum privsep_procid, int);
>  struct imsgev *
>        proc_iev(struct privsep *, enum privsep_procid, int);
> +int   proc_flush_imsg(struct privsep *, enum privsep_procid, int);
>  
>  /* traphandler.c */
> -pid_t         traphandler(struct privsep *, struct privsep_proc *);
> +void  traphandler(struct privsep *, struct privsep_proc *);
>  void  traphandler_shutdown(void);
>  int   snmpd_dispatch_traphandler(int, struct privsep_proc *, struct imsg *);
>  void  trapcmd_free(struct trapcmd *);
> Index: snmpe.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/snmpe.c,v
> retrieving revision 1.42
> diff -u -p -r1.42 snmpe.c
> --- snmpe.c   16 Aug 2016 18:41:57 -0000      1.42
> +++ snmpe.c   14 Oct 2016 15:41:11 -0000
> @@ -60,7 +60,7 @@ static struct privsep_proc procs[] = {
>       { "parent",     PROC_PARENT,    snmpe_dispatch_parent }
>  };
>  
> -pid_t
> +void
>  snmpe(struct privsep *ps, struct privsep_proc *p)
>  {
>  #ifdef DEBUG
> @@ -81,7 +81,7 @@ snmpe(struct privsep *ps, struct privsep
>       if ((env->sc_sock = snmpe_bind(&env->sc_address)) == -1)
>               fatalx("snmpe: failed to bind SNMP UDP socket");
>  
> -     return (proc_run(ps, p, procs, nitems(procs), snmpe_init, NULL));
> +     proc_run(ps, p, procs, nitems(procs), snmpe_init, NULL);
>  }
>  
>  /* ARGSUSED */
> Index: traphandler.c
> ===================================================================
> RCS file: /home/obsdcvs/src/usr.sbin/snmpd/traphandler.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 traphandler.c
> --- traphandler.c     16 Aug 2016 18:41:57 -0000      1.5
> +++ traphandler.c     21 Oct 2016 10:48:43 -0000
> @@ -72,7 +72,7 @@ static struct privsep_proc procs[] = {
>       { "parent",     PROC_PARENT,    traphandler_dispatch_parent }
>  };
>  
> -pid_t
> +void
>  traphandler(struct privsep *ps, struct privsep_proc *p)
>  {
>       struct snmpd            *env = ps->ps_env;
> @@ -81,8 +81,7 @@ traphandler(struct privsep *ps, struct p
>           (trapsock = traphandler_bind(&env->sc_address)) == -1)
>               fatal("could not create trap listener socket");
>  
> -     return (proc_run(ps, p, procs, nitems(procs), traphandler_init,
> -         NULL));
> +     proc_run(ps, p, procs, nitems(procs), traphandler_init, NULL);
>  }
>  
>  void
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply via email to