privsep: Reduce fd use

On start close all FD's above stderr.
Close some fd's we don't need in processes spawned from priv.
Ensure we init some FD's to -1 to ensure we don't close stdin.
If DEBUG_FD is defined, we log FD's opened by pid.
Audit process FD usage and document it so I don't forget it.

Fixes #316.

diff --git a/src/dhcp6.c b/src/dhcp6.c
index 58ec4eb0adb0c20603be04256a00169d2cae6191..037cc242bf0f996cc4158480d68543a956bdebe8 100644 (file)
--- a/src/dhcp6.c
+++ b/src/dhcp6.c
@@ -3791,7 +3791,7 @@ dhcp6_openraw(void)
 {
        int fd, v;
 
-       fd = socket(PF_INET6, SOCK_RAW | SOCK_CXNB, IPPROTO_UDP);
+       fd = xsocket(PF_INET6, SOCK_RAW | SOCK_CXNB, IPPROTO_UDP);
        if (fd == -1)
                return -1;
 

diff --git a/src/dhcpcd.c b/src/dhcpcd.c
index d9e071577e2f756e3ad117f5a9f66660ee983c19..ccc8fbcf99d8b3242209d0d2c831ce8785f57986 100644 (file)
--- a/src/dhcpcd.c
+++ b/src/dhcpcd.c
@@ -1981,6 +1981,7 @@ main(int argc, char **argv, char **envp)
        }
 
        memset(&ctx, 0, sizeof(ctx));
+       closefrom(STDERR_FILENO + 1);
 
        ifo = NULL;
        ctx.cffile = CONFIG;
@@ -2010,7 +2011,7 @@ main(int argc, char **argv, char **envp)
        ctx.dhcp6_wfd = -1;
 #endif
 #ifdef PRIVSEP
-       ctx.ps_log_fd = -1;
+       ctx.ps_log_fd = ctx.ps_log_root_fd = -1;
        TAILQ_INIT(&ctx.ps_processes);
 #endif
 
@@ -2461,9 +2462,14 @@ printpidfile:
                goto run_loop;
        }
 
+#ifdef DEBUG_FD
+       loginfox("forkfd %d", ctx.fork_fd);
+#endif
+
        /* We have now forked, setsid, forked once more.
         * From this point on, we are the controlling daemon. */
        logdebugx("spawned manager process on PID %d", getpid());
+
 start_manager:
        ctx.options |= DHCPCD_STARTED;
        if ((pid = pidfile_lock(ctx.pidfile)) != 0) {
@@ -2681,10 +2687,6 @@ exit1:
        free(ctx.script_env);
        rt_dispose(&ctx);
        free(ctx.duid);
-       if (ctx.link_fd != -1) {
-               eloop_event_delete(ctx.eloop, ctx.link_fd);
-               close(ctx.link_fd);
-       }
        if_closesockets(&ctx);
        free_globals(&ctx);
 #ifdef INET6

diff --git a/src/if-bsd.c b/src/if-bsd.c
index bab33c2ca6a9df1db6048fe9faaa49e230d76e8a..9ebe36a88fd3179f78c632ec23727df2404bff11 100644 (file)
--- a/src/if-bsd.c
+++ b/src/if-bsd.c
@@ -197,6 +197,18 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
            &n, sizeof(n)) == -1)
                logerr("%s: SO_USELOOPBACK", __func__);
 
+#ifdef PRIVSEP
+       if (ctx->options & DHCPCD_PRIVSEPROOT) {
+               /* We only want to write to this socket, so set
+                * a small as possible buffer size. */
+               socklen_t smallbuf = 1;
+
+               if (setsockopt(ctx->link_fd, SOL_SOCKET, SO_RCVBUF,
+                   &smallbuf, (socklen_t)sizeof(smallbuf)) == -1)
+                       logerr("%s: setsockopt(SO_RCVBUF)", __func__);
+       }
+#endif
+
 #if defined(RO_MSGFILTER)
        if (setsockopt(ctx->link_fd, PF_ROUTE, RO_MSGFILTER,
            &msgfilter, sizeof(msgfilter)) == -1)
@@ -220,9 +232,8 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
                ps_rights_limit_fd_sockopt(ctx->link_fd);
 #endif
 
-
 #if defined(SIOCALIFADDR) && defined(IFLR_ACTIVE) /*NetBSD */
-       priv->pf_link_fd = socket(PF_LINK, SOCK_DGRAM, 0);
+       priv->pf_link_fd = xsocket(PF_LINK, SOCK_DGRAM, 0);
        if (priv->pf_link_fd == -1)
                logerr("%s: socket(PF_LINK)", __func__);
 #endif
@@ -235,13 +246,20 @@ if_closesockets_os(struct dhcpcd_ctx *ctx)
        struct priv *priv;
 
        priv = (struct priv *)ctx->priv;
+       if (priv == NULL)
+               return;
+
 #ifdef INET6
-       if (priv->pf_inet6_fd != -1)
+       if (priv->pf_inet6_fd != -1) {
                close(priv->pf_inet6_fd);
+               priv->pf_inet6_fd = -1;
+       }
 #endif
 #if defined(SIOCALIFADDR) && defined(IFLR_ACTIVE) /*NetBSD */
-       if (priv->pf_link_fd != -1)
+       if (priv->pf_link_fd != -1) {
                close(priv->pf_link_fd);
+               priv->pf_link_fd = -1;
+       }
 #endif
        free(priv);
        ctx->priv = NULL;

diff --git a/src/if-linux-wext.c b/src/if-linux-wext.c
index 40498571df023802232dbb611519eb161eb129f8..88545dc9773fd7d9c34d33c8be4d927ba93ca8fa 100644 (file)
--- a/src/if-linux-wext.c
+++ b/src/if-linux-wext.c
@@ -69,7 +69,7 @@ if_getssid_wext(const char *ifname, uint8_t *ssid)
        int s, retval;
        struct iwreq iwr;
 
-       if ((s = socket(PF_INET, SOCK_DGRAM, 0)) == -1)
+       if ((s = xsocket(PF_INET, SOCK_DGRAM, 0)) == -1)
                return -1;
        memset(&iwr, 0, sizeof(iwr));
        strlcpy(iwr.ifr_name, ifname, sizeof(iwr.ifr_name));

diff --git a/src/if-linux.c b/src/if-linux.c
index e18258210c169c22f413b3c9303c7af10443a118..2a599516764626ac4c5261180f43ffe2de989541 100644 (file)
--- a/src/if-linux.c
+++ b/src/if-linux.c
@@ -500,15 +500,22 @@ setup_priv:
 void
 if_closesockets_os(struct dhcpcd_ctx *ctx)
 {
-       struct priv *priv;
+       struct priv *priv = ctx->priv;
+
+       if (priv == NULL)
+               return;
 
-       if (ctx->priv != NULL) {
-               priv = (struct priv *)ctx->priv;
-               if (priv->route_fd != -1)
-                       close(priv->route_fd);
-               if (priv->generic_fd != -1)
-                       close(priv->generic_fd);
+       if (priv->route_fd != -1) {
+               close(priv->route_fd);
+               priv->route_fd = -1;
        }
+       if (priv->generic_fd != -1) {
+               close(priv->generic_fd);
+               priv->generic_fd = -1;
+       }
+
+       free(priv);
+       ctx->priv = NULL;
 }
 
 int

diff --git a/src/if-options.c b/src/if-options.c
index c57f6a31fe2c71fdb812279d3bac239b05aec8c4..f4f55204b13afcdefdc42533d1e234fb469dec7e 100644 (file)
--- a/src/if-options.c
+++ b/src/if-options.c
@@ -2465,7 +2465,7 @@ read_config(struct dhcpcd_ctx *ctx,
                default_options |= DHCPCD_CONFIGURE | DHCPCD_DAEMONISE |
                    DHCPCD_GATEWAY;
 #ifdef INET
-               skip = socket(PF_INET, SOCK_DGRAM, 0);
+               skip = xsocket(PF_INET, SOCK_DGRAM, 0);
                if (skip != -1) {
                        close(skip);
                        default_options |= DHCPCD_IPV4 | DHCPCD_ARP |
@@ -2473,7 +2473,7 @@ read_config(struct dhcpcd_ctx *ctx,
                }
 #endif
 #ifdef INET6
-               skip = socket(PF_INET6, SOCK_DGRAM, 0);
+               skip = xsocket(PF_INET6, SOCK_DGRAM, 0);
                if (skip != -1) {
                        close(skip);
                        default_options |= DHCPCD_IPV6 | DHCPCD_IPV6RS |

diff --git a/src/if-sun.c b/src/if-sun.c
index 0a60368edb5e63287c5398b9570ea28bb5584913..37d0096196727f7f9fe5053fa43c744eba459d59 100644 (file)
--- a/src/if-sun.c
+++ b/src/if-sun.c
@@ -154,7 +154,7 @@ if_opensockets_os(struct dhcpcd_ctx *ctx)
         * We will fail noisily elsewhere anyway. */
 #endif
 
-       ctx->link_fd = socket(PF_ROUTE,
+       ctx->link_fd = xsocket(PF_ROUTE,
            SOCK_RAW | SOCK_CLOEXEC | SOCK_NONBLOCK, 0);
 
        if (ctx->link_fd == -1) {
@@ -180,12 +180,13 @@ if_closesockets_os(struct dhcpcd_ctx *ctx)
        struct priv             *priv;
 
        priv = (struct priv *)ctx->priv;
-       if (priv->pf_inet6_fd != -1)
+       if (priv && priv->pf_inet6_fd != -1)
                close(priv->pf_inet6_fd);
 #endif
 
        /* each interface should have closed itself */
        free(ctx->priv);
+       ctx->priv = NULL;
 }
 
 int
@@ -727,7 +728,7 @@ if_route_get(struct dhcpcd_ctx *ctx, struct rt *rt)
 
        if_route0(ctx, &rtm, RTM_GET, rt);
        rt = NULL;
-       s = socket(PF_ROUTE, SOCK_RAW | SOCK_CLOEXEC, 0);
+       s = xsocket(PF_ROUTE, SOCK_RAW | SOCK_CLOEXEC, 0);
        if (s == -1)
                return NULL;
        if (write(s, &rtm, rtm.hdr.rtm_msglen) == -1)

diff --git a/src/if.c b/src/if.c
index 3446a9f786cc07ed20e686b9a34a4bca19451ccd..3ed9f6b38efb43026de3c78e01cb7f47244efa87 100644 (file)
--- a/src/if.c
+++ b/src/if.c
@@ -132,17 +132,18 @@ void
 if_closesockets(struct dhcpcd_ctx *ctx)
 {
 
-       if (ctx->pf_inet_fd != -1)
-               close(ctx->pf_inet_fd);
-#ifdef PF_LINK
-       if (ctx->pf_link_fd != -1)
-               close(ctx->pf_link_fd);
-#endif
+       if (ctx->link_fd != -1) {
+               eloop_event_delete(ctx->eloop, ctx->link_fd);
+               close(ctx->link_fd);
+               ctx->link_fd = -1;
+       }
 
-       if (ctx->priv) {
-               if_closesockets_os(ctx);
-               free(ctx->priv);
+       if (ctx->pf_inet_fd != -1) {
+               close(ctx->pf_inet_fd);
+               ctx->pf_inet_fd = -1;
        }
+
+       if_closesockets_os(ctx);
 }
 
 int
@@ -982,6 +983,10 @@ xsocket(int domain, int type, int protocol)
 
        if ((s = socket(domain, type, protocol)) == -1)
                return -1;
+#ifdef DEBUG_FD
+       logerrx("pid %d fd=%d domain=%d type=%d protocol=%d",
+           getpid(), s, domain, type, protocol);
+#endif
 
 #ifndef HAVE_SOCK_CLOEXEC
        if ((xtype & SOCK_CLOEXEC) && ((xflags = fcntl(s, F_GETFD)) == -1 ||
@@ -1023,6 +1028,10 @@ xsocketpair(int domain, int type, int protocol, int fd[2])
        if ((s = socketpair(domain, type, protocol, fd)) == -1)
                return -1;
 
+#ifdef DEBUG_FD
+       logerrx("pid %d fd[0]=%d fd[1]=%d", getpid(), fd[0], fd[1]);
+#endif
+
 #ifndef HAVE_SOCK_CLOEXEC
        if ((xtype & SOCK_CLOEXEC) && ((xflags = fcntl(fd[0], F_GETFD)) == -1 ||
            fcntl(fd[0], F_SETFD, xflags | FD_CLOEXEC) == -1))

diff --git a/src/logerr.c b/src/logerr.c
index 883b2b19a6f4b09dd47dbe00b5cf6a6975992f67..319d6f48e6ce2cdb8f505061cfac27e0d1a3539d 100644 (file)
--- a/src/logerr.c
+++ b/src/logerr.c
@@ -376,6 +376,8 @@ logsetfd(int fd)
        struct logctx *ctx = &_logctx;
 
        ctx->log_fd = fd;
+       if (fd != -1)
+               closelog();
 #ifndef SMALL
        if (fd != -1 && ctx->log_file != NULL) {
                fclose(ctx->log_file);

diff --git a/src/privsep-bpf.c b/src/privsep-bpf.c
index 989400902d7879e9367109daa46429223366d4a3..742865351abfc139afad357ba6526f11678aa362 100644 (file)
--- a/src/privsep-bpf.c
+++ b/src/privsep-bpf.c
@@ -53,6 +53,8 @@
 #include "logerr.h"
 #include "privsep.h"
 
+/* We expect to have open 3 SEQPACKET and one RAW fd */
+
 static void
 ps_bpf_recvbpf(void *arg, unsigned short events)
 {
@@ -160,6 +162,9 @@ ps_bpf_start_bpf(struct ps_process *psp)
        ps_freeprocesses(ctx, psp);
 
        psp->psp_bpf = bpf_open(&psp->psp_ifp, psp->psp_filter, ia);
+#ifdef DEBUG_FD
+       logdebugx("pid %d bpf_fd=%d", getpid(), psp->psp_bpf->bpf_fd);
+#endif
        if (psp->psp_bpf == NULL)
                logerr("%s: bpf_open",__func__);
 #ifdef PRIVSEP_RIGHTS

diff --git a/src/privsep-control.c b/src/privsep-control.c
index 40bfb16469e7477d808beda9891b5af1d55411ba..5bce41746d3f7b0c51450f84ed95aea108aa5a0d 100644 (file)
--- a/src/privsep-control.c
+++ b/src/privsep-control.c
@@ -36,6 +36,8 @@
 #include "logerr.h"
 #include "privsep.h"
 
+/* We expect to have open 2 SEQPACKET, 2 STREAM and 2 file STREAM fds */
+
 static int
 ps_ctl_startcb(struct ps_process *psp)
 {
@@ -217,14 +219,16 @@ ps_ctl_start(struct dhcpcd_ctx *ctx)
                .psi_cmd = PS_CTL,
        };
        struct ps_process *psp;
-       int data_fd[2], listen_fd[2];
+       int work_fd[2], listen_fd[2];
        pid_t pid;
 
-       if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, data_fd) == -1 ||
+       if_closesockets(ctx);
+
+       if (xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, work_fd) == -1 ||
            xsocketpair(AF_UNIX, SOCK_STREAM | SOCK_CXNB, 0, listen_fd) == -1)
                return -1;
 #ifdef PRIVSEP_RIGHTS
-       if (ps_rights_limit_fdpair(data_fd) == -1 ||
+       if (ps_rights_limit_fdpair(work_fd) == -1 ||
            ps_rights_limit_fdpair(listen_fd) == -1)
                return -1;
 #endif
@@ -237,24 +241,25 @@ ps_ctl_start(struct dhcpcd_ctx *ctx)
        if (pid == -1)
                return -1;
        else if (pid != 0) {
-               psp->psp_work_fd = data_fd[1];
-               close(data_fd[0]);
+               psp->psp_work_fd = work_fd[0];
+               close(work_fd[1]);
+               close(listen_fd[1]);
                ctx->ps_control = control_new(ctx,
-                   listen_fd[1], FD_SENDLEN | FD_LISTEN);
+                   listen_fd[0], FD_SENDLEN | FD_LISTEN);
                if (ctx->ps_control == NULL)
                        return -1;
-               close(listen_fd[0]);
                return pid;
        }
 
-       psp->psp_work_fd = data_fd[0];
-       close(data_fd[1]);
+       close(work_fd[0]);
+       close(listen_fd[0]);
+
+       psp->psp_work_fd = work_fd[1];
        if (eloop_event_add(ctx->eloop, psp->psp_work_fd, ELE_READ,
            ps_ctl_recv, ctx) == -1)
                return -1;
 
-       ctx->ps_control = control_new(ctx, listen_fd[0], 0);
-       close(listen_fd[1]);
+       ctx->ps_control = control_new(ctx, listen_fd[1], 0);
        if (ctx->ps_control == NULL)
                return -1;
        if (eloop_event_add(ctx->eloop, ctx->ps_control->fd, ELE_READ,

diff --git a/src/privsep-inet.c b/src/privsep-inet.c
index 5895a5730c1f097c4c8ee0b2235cc854b02f20f4..919fe9540b0ab3b2f4651e801a4f0efbe1283417 100644 (file)
--- a/src/privsep-inet.c
+++ b/src/privsep-inet.c
@@ -47,6 +47,8 @@
 #include "logerr.h"
 #include "privsep.h"
 
+/* We expect to have open 2 SEQPACKET, 1 udp, 1 udp6 and 1 raw6 fds */
+
 #ifdef INET
 static void
 ps_inet_recvbootp(void *arg, unsigned short events)

diff --git a/src/privsep-root.c b/src/privsep-root.c
index eb2b3eef687a7319048d8c6d93ca14b03f20367b..ed4bb699769ef1f1ba936668316129b05d6be5b9 100644 (file)
--- a/src/privsep-root.c
+++ b/src/privsep-root.c
@@ -259,7 +259,7 @@ ps_root_doioctl(unsigned long req, void *data, size_t len)
                return -1;
        }
 
-       s = socket(PF_INET, SOCK_DGRAM, 0);
+       s = xsocket(PF_INET, SOCK_DGRAM, 0);
        if (s != -1)
 #ifdef IOCTL_REQUEST_TYPE
        {
@@ -685,17 +685,6 @@ ps_root_startcb(struct ps_process *psp)
 
        if (if_opensockets(ctx) == -1)
                logerr("%s: if_opensockets", __func__);
-#ifdef BSD
-       else {
-               /* We only want to write to this socket, so set
-                * a small as possible buffer size. */
-               socklen_t smallbuf = 1;
-
-               if (setsockopt(ctx->link_fd, SOL_SOCKET, SO_RCVBUF,
-                   &smallbuf, (socklen_t)sizeof(smallbuf)) == -1)
-                       logerr("%s: setsockopt(SO_RCVBUF)", __func__);
-       }
-#endif
 
        /* Open network sockets for sending.
         * This is a small bit wasteful for non sandboxed OS's
@@ -899,7 +888,7 @@ ps_root_start(struct dhcpcd_ctx *ctx)
                return -1;
 #endif
 
-       if (socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, datafd) == -1)
+       if (xsocketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CXNB, 0, datafd) == -1)
                return -1;
        if (ps_setbuf_fdpair(datafd) == -1)
                return -1;
@@ -937,6 +926,32 @@ ps_root_start(struct dhcpcd_ctx *ctx)
        return pid;
 }
 
+void
+ps_root_close(struct dhcpcd_ctx *ctx)
+{
+
+       if_closesockets(ctx);
+
+#ifdef INET
+       if (ctx->udp_wfd != -1) {
+               close(ctx->udp_wfd);
+               ctx->udp_wfd = -1;
+       }
+#endif
+#ifdef INET6
+       if (ctx->nd_fd != -1) {
+               close(ctx->nd_fd);
+               ctx->nd_fd = -1;
+       }
+#endif
+#ifdef DHCP6
+       if (ctx->dhcp6_wfd != -1) {
+               close(ctx->dhcp6_wfd);
+               ctx->dhcp6_wfd = -1;
+       }
+#endif
+}
+
 int
 ps_root_stop(struct dhcpcd_ctx *ctx)
 {

diff --git a/src/privsep-root.h b/src/privsep-root.h
index ccb2f485822d50b51c11b3e76b43f31af29da307..ce824db97664bbd0b3a23e1d690112cc7effaa91 100644 (file)
--- a/src/privsep-root.h
+++ b/src/privsep-root.h
@@ -36,6 +36,7 @@
 #endif
 
 pid_t ps_root_start(struct dhcpcd_ctx *ctx);
+void ps_root_close(struct dhcpcd_ctx *ctx);
 int ps_root_stop(struct dhcpcd_ctx *ctx);
 void ps_root_signalcb(int, void *);
 

diff --git a/src/privsep-sun.c b/src/privsep-sun.c
index a645d15dc25226ff5846a554afd06dd116a15e71..6e539c198663fc2ec8f88facb5cfb53ca41c7d22 100644 (file)
--- a/src/privsep-sun.c
+++ b/src/privsep-sun.c
@@ -45,7 +45,7 @@ ps_root_doioctl6(unsigned long req, void *data, size_t len)
 {
        int s, err;
 
-       s = socket(PF_INET6, SOCK_DGRAM, 0);
+       s = xsocket(PF_INET6, SOCK_DGRAM, 0);
        if (s != -1)
                err = ioctl(s, req, data, len);
        else
@@ -63,7 +63,7 @@ ps_root_doroute(void *data, size_t len)
        int s;
        ssize_t err;
 
-       s = socket(PF_ROUTE, SOCK_RAW, 0);
+       s = xsocket(PF_ROUTE, SOCK_RAW, 0);
        if (s != -1)
                err = write(s, data, len);
        else

diff --git a/src/privsep.c b/src/privsep.c
index 4cca12ee379e3bba2adfd83c75448553b8313d73..4cccfbdb2b874a759e3802019f71f60e82a048a7 100644 (file)
--- a/src/privsep.c
+++ b/src/privsep.c
@@ -408,15 +408,23 @@ ps_startprocess(struct ps_process *psp,
                return pid;
        }
 
+       /* If we are not the root process, close un-needed stuff. */
+       if (ctx->ps_root != psp) {
+               ps_root_close(ctx);
 #ifdef PLUGIN_DEV
-       /* If we are not the root process, stop listening to devices. */
-       if (ctx->ps_root != psp)
                dev_stop(ctx);
 #endif
+       }
 
        ctx->options |= DHCPCD_FORKED;
        if (ctx->ps_log_fd != -1)
                logsetfd(ctx->ps_log_fd);
+
+#ifdef DEBUG_FD
+       logerrx("pid %d log_fd=%d data_fd=%d psp_fd=%d",
+           getpid(), ctx->ps_log_fd, ctx->ps_data_fd, psp->psp_fd);
+#endif
+
        eloop_clear(ctx->eloop, -1);
        eloop_forked(ctx->eloop);
        eloop_signal_set_cb(ctx->eloop,
@@ -459,18 +467,6 @@ ps_startprocess(struct ps_process *psp,
 #endif
        }
 
-       if (ctx->ps_inet != psp)
-               ctx->ps_inet = NULL;
-       if (ctx->ps_ctl != psp)
-               ctx->ps_ctl = NULL;
-
-#if 0
-       char buf[1024];
-       errno = 0;
-       ssize_t xx = recv(psp->psp_fd, buf, sizeof(buf), MSG_PEEK);
-       logerr("pid %d test fd %d recv peek %zd", getpid(), psp->psp_fd, xx);
-#endif
-
        if (eloop_event_add(ctx->eloop, psp->psp_fd, ELE_READ,
            recv_msg, psp) == -1)
        {
@@ -1215,6 +1211,7 @@ ps_newprocess(struct dhcpcd_ctx *ctx, struct ps_id *psid)
                return NULL;
        psp->psp_ctx = ctx;
        memcpy(&psp->psp_id, psid, sizeof(psp->psp_id));
+       psp->psp_fd = -1;
        psp->psp_work_fd = -1;
 #ifdef HAVE_CAPSICUM
        psp->psp_pfd = -1;