The problems is that ldpd exits with a pledge violation. However, it happens
when ldpd is terminating during ldpe_shutdown().

OpenBSD 6.7 (GENERIC.MP) #182: Thu May  7 11:11:58 MDT 2020
        dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP

A demonstration of the problem can be achieved on a newly installed OpenBSD
6.7 as follows:

    # (umask -S 0177; touch /etc/ldpd.conf); ldpd -dv & p=$!; sleep 1; kill $p
    [1] 38081                     
    startup                         
    accept_add: accepting on fd 4    
    lde add fec ....
    ....
    kernel routing table decoupled
    accept_del: 4 removed from queue
    waiting for children to terminate
    label decision engine exiting
    ldp engine terminated; signal 6
    terminating

The "cpath" pledge(2) was removed from the engine process, from what I
understood from the commit message, on the false premise that the socket is
unlinked by the main process.

The fix, is I see it, would be to either 1) actually unlink the socket in
the main process or 2) add "cpath" back to the engine process.  2) seems
like a bad option, but 1) makes me doubt my assumptions since this story
contains contradicting statements.

So, I propose that the the call of control_cleanup() is moved from
ldpe_shutdown() to ldpd_shutdown().  This changes the order in which the
unlink(2) call happens relatively to the other steps in ldpe_shutdown(),
however this should be of no concern as I see it.

diff refs/heads/master 098bb599a09e458fcda8f1c5c8b82ddbb0d6970e
blob - b867d7961dc964ad7f7e31cbbe1cd6903d87364b
blob + 99bdd2d52f4898844de5e704448e0e2e41158d6c
--- usr.sbin/ldpd/ldpd.c
+++ usr.sbin/ldpd/ldpd.c
@@ -34,13 +34,13 @@
 #include "ldpd.h"
 #include "ldpe.h"
 #include "lde.h"
+#include "control.h"
 #include "log.h"
 
 static void             main_sig_handler(int, short, void *);
 static __dead void      usage(void);
 static __dead void      ldpd_shutdown(void);
-static pid_t            start_child(enum ldpd_process, char *, int, int, int,
-                           char *);
+static pid_t            start_child(enum ldpd_process, char *, int, int, int);
 static void             main_dispatch_ldpe(int, short, void *);
 static void             main_dispatch_lde(int, short, void *);
 static int              main_imsg_compose_both(enum imsg_type, void *,
@@ -71,6 +71,7 @@ static struct imsgev  *iev_ldpe;
 static struct imsgev   *iev_lde;
 static pid_t            ldpe_pid;
 static pid_t            lde_pid;
+static char            *sockname;
 
 /* ARGSUSED */
 static void
@@ -111,7 +112,6 @@ main(int argc, char *argv[])
        char                    *saved_argv0;
        int                      ch;
        int                      debug = 0, lflag = 0, eflag = 0;
-       char                    *sockname;
        int                      pipe_parent2ldpe[2];
        int                      pipe_parent2lde[2];
 
@@ -216,11 +216,9 @@ main(int argc, char *argv[])
 
        /* start children */
        lde_pid = start_child(PROC_LDE_ENGINE, saved_argv0,
-           pipe_parent2lde[1], debug, global.cmd_opts & LDPD_OPT_VERBOSE,
-           NULL);
+           pipe_parent2lde[1], debug, global.cmd_opts & LDPD_OPT_VERBOSE);
        ldpe_pid = start_child(PROC_LDP_ENGINE, saved_argv0,
-           pipe_parent2ldpe[1], debug, global.cmd_opts & LDPD_OPT_VERBOSE,
-           sockname);
+           pipe_parent2ldpe[1], debug, global.cmd_opts & LDPD_OPT_VERBOSE);
 
        if (unveil("/", "r") == -1)
                fatal("unveil");
@@ -297,6 +295,7 @@ ldpd_shutdown(void)
        close(iev_lde->ibuf.fd);
 
        kr_shutdown();
+       control_cleanup(sockname);
        config_clear(ldpd_conf);
 
        log_debug("waiting for children to terminate");
@@ -319,8 +318,7 @@ ldpd_shutdown(void)
 }
 
 static pid_t
-start_child(enum ldpd_process p, char *argv0, int fd, int debug, int verbose,
-    char *sockname)
+start_child(enum ldpd_process p, char *argv0, int fd, int debug, int verbose)
 {
        char    *argv[5];
        int      argc = 0;
@@ -351,16 +349,14 @@ start_child(enum ldpd_process p, char *argv0, int fd, 
                break;
        case PROC_LDP_ENGINE:
                argv[argc++] = "-E";
+               argv[argc++] = "-s";
+               argv[argc++] = sockname;
                break;
        }
        if (debug)
                argv[argc++] = "-d";
        if (verbose)
                argv[argc++] = "-v";
-       if (sockname) {
-               argv[argc++] = "-s";
-               argv[argc++] = sockname;
-       }
        argv[argc++] = NULL;
 
        execvp(argv0, argv);
blob - 6200d552882ca678e391c59922ccf358439ddac4
blob + 2b769ebd8a42ef11711b46320759741559562ed9
--- usr.sbin/ldpd/ldpe.c
+++ usr.sbin/ldpd/ldpe.c
@@ -171,7 +171,6 @@ ldpe_shutdown(void)
        msgbuf_clear(&iev_main->ibuf.w);
        close(iev_main->ibuf.fd);
 
-       control_cleanup(global.csock);
        config_clear(leconf);
 
        if (sysdep.no_pfkey == 0) {

Reply via email to