From 439d9df122c91a884883815c2d755f50f076bc36 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 10:49:31 +0200 Subject: [PATCH 1/7] Set CLOEXEC flag to prevent leaking descriptors Both the new initctl API and the new pidfile watcher plugin failed to set CLOEXEC on their sockets. This caused forked-off and exec()'d children to inherit all these descriptors. Signed-off-by: Joachim Nilsson --- api.c | 2 +- plugins/pidfile.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api.c b/api.c index 03ceec34..37725dd0 100644 --- a/api.c +++ b/api.c @@ -346,7 +346,7 @@ int api_init(uev_ctx_t *ctx) .sun_path = INIT_SOCKET, }; - sd = socket(AF_UNIX, SOCK_STREAM, 0); + sd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0); if (-1 == sd) { _pe("Failed starting external API socket"); return 1; diff --git a/plugins/pidfile.c b/plugins/pidfile.c index 7fdb4f6e..f9db3126 100644 --- a/plugins/pidfile.c +++ b/plugins/pidfile.c @@ -104,7 +104,7 @@ static plugin_t plugin = { PLUGIN_INIT(plugin_init) { - pidfile_ctx.fd = inotify_init(); + pidfile_ctx.fd = inotify_init1(IN_NONBLOCK | IN_CLOEXEC); if (pidfile_ctx.fd < 0) { _pe("inotify_init()"); return; From 06d1e90bacff9a1b3a97d100b725459ff50db6f5 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 10:53:20 +0200 Subject: [PATCH 2/7] Do not just mark deleted SVC free, clear memory Signed-off-by: Joachim Nilsson --- svc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/svc.c b/svc.c index fb241b22..631da95f 100644 --- a/svc.c +++ b/svc.c @@ -97,7 +97,9 @@ svc_t *svc_new(char *cmd, int id, int type) */ int svc_del(svc_t *svc) { - svc->type = SVC_TYPE_FREE; + /* Clear any old references and set SVC_TYPE_FREE */ + memset(svc, 0, sizeof(*svc)); + return 0; } From 9a03adce5aad38e7e6a201b3bdf2aed78f6c1964 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 10:54:31 +0200 Subject: [PATCH 3/7] Do not assume new fd is 0, dup2() with fd instead Signed-off-by: Joachim Nilsson --- exec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exec.c b/exec.c index 5ed08e73..76c937bb 100644 --- a/exec.c +++ b/exec.c @@ -245,9 +245,9 @@ pid_t run_getty(char *cmd, char *args[], int console) if (fd != STDIN_FILENO) exit(1); - dup2(0, STDIN_FILENO); - dup2(0, STDOUT_FILENO); - dup2(0, STDERR_FILENO); + dup2(fd, STDIN_FILENO); + dup2(fd, STDOUT_FILENO); + dup2(fd, STDERR_FILENO); prctl(PR_SET_NAME, "console", 0, 0, 0); } From 7e91cb4afc5ba213db8a04544a186c38a7131c83 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 10:58:13 +0200 Subject: [PATCH 4/7] Minor log message improvements Signed-off-by: Joachim Nilsson --- inetd.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/inetd.c b/inetd.c index 52671aca..a537b733 100644 --- a/inetd.c +++ b/inetd.c @@ -127,8 +127,7 @@ static int get_stdin(svc_t *svc) } if (!inetd_is_allowed(&svc->inetd, ifname)) { - FLOG_INFO("Service %s on port %d not allowed from interface %s.", - svc->inetd.name, svc->inetd.port, ifname); + FLOG_INFO("Service %s on %s:%d is not allowed.", svc->inetd.name, ifname, svc->inetd.port); if (svc->inetd.type == SOCK_STREAM) close(stdin); @@ -144,10 +143,10 @@ static void socket_cb(uev_t *w, void *arg, int UNUSED(events)) svc_t *svc = (svc_t *)arg, *task; int stdin; + _d("Got event on %s socket ...", svc->cmd); stdin = get_stdin(svc); if (stdin < 0) { - FLOG_ERROR("%s: Unable to accept incoming connection", - svc->cmd); + FLOG_ERROR("%s: Unable to accept incoming connection", svc->cmd); return; } @@ -190,7 +189,7 @@ static int spawn_socket(inetd_t *inetd) return -EINVAL; } - _d("Spawning server socket for inetd %s ...", inetd->name); + _d("Spawning server socket for inetd %s, type %s ...", inetd->name, inetd->type == SOCK_STREAM ? "stream" : "dgram"); sd = socket(AF_INET, inetd->type | SOCK_NONBLOCK | SOCK_CLOEXEC, inetd->proto); if (-1 == sd) { FLOG_PERROR("Failed opening inetd socket type %d proto %d", inetd->type, inetd->proto); From efd265c9c8a6411bfc128742a8e2a993e4044a72 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 11:32:16 +0200 Subject: [PATCH 5/7] Fix "broken pipe" problem for UDP inetd services This patch fixes several issues related to the problem of launching UDP inetd services: - Make sure to disable O_NONBLOCK on the socket before passing it to the inetd service, that's what is expected. This goes for both UDP and TCP (accepted) connections -- there is no guarantee even for TCP sockets that they do *not* inherit the parent socket's flags. - Mark the UDP inetd parent's SVC as busy and stop its watcher before starting the child task to service the connection. The watcher must be stopped or multiple child tasks may be spawned! - Only copy the most relevant parts to the UDP task. In particular must *not* copy the libuEv watcher data to the task! - With the child task done and the parent inetd service unblocked we must restore O_NONBLOCK to the socket before restarting the watcher When a UDP inetd service connection is handled we create a child task to service the request, meanwhile we block the parent service. There was a problem with blocking the parent since we called inetd_stop(), which both did a shutdown() and close() of the socket ... this patch introduces separate handling of blocked (busy) inetd services in the inetd_stop() function and also make sure to handle restarting the parent watcher in inetd_start() when the child task is done. Signed-off-by: Joachim Nilsson --- inetd.c | 59 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/inetd.c b/inetd.c index a537b733..4d8e8767 100644 --- a/inetd.c +++ b/inetd.c @@ -150,6 +150,15 @@ static void socket_cb(uev_t *w, void *arg, int UNUSED(events)) return; } + /* + * Make sure to disable O_NONBLOCK on the descriptor before + * passing it to the inetd service, that's what is expected. + */ + if (fcntl(stdin, F_SETFL, fcntl(stdin, F_GETFL, 0) & ~O_NONBLOCK) < 0) { + FLOG_ERROR("Failed disabling non-blocking on %s socket", svc->cmd); + return; + } + task = svc_new(svc->cmd, svc->inetd.next_id++, SVC_TYPE_INETD_CONN); if (!task) { FLOG_ERROR("%s: Unable to allocate service for inetd client", @@ -157,9 +166,19 @@ static void socket_cb(uev_t *w, void *arg, int UNUSED(events)) return; } - /* Copy inherited attributes from inetd */ + if (!svc->inetd.forking) { + svc->block = SVC_BLOCK_INETD_BUSY; + service_step(svc); + } + + /* Copy inherited attributes from inetd service's svc */ task->runlevels = svc->runlevels; - task->inetd = svc->inetd; + + /* Only copy the most relevant parts of inetd, in particular we + * must *not* copy the watcher data to the clone! */ + task->inetd.svc = svc; + task->inetd.type = svc->inetd.type; + memcpy(task->cond, svc->cond, sizeof(task->cond)); memcpy(task->username, svc->username, sizeof(task->username)); memcpy(task->group, svc->group, sizeof(task->group)); @@ -168,11 +187,6 @@ static void socket_cb(uev_t *w, void *arg, int UNUSED(events)) task->stdin = stdin; service_step(task); - - if (!svc->inetd.forking) { - svc->block = SVC_BLOCK_INETD_BUSY; - service_step(svc); - } } /* Launch Inet socket for service. @@ -231,19 +245,40 @@ static int spawn_socket(inetd_t *inetd) int inetd_start(inetd_t *inetd) { - if (inetd->watcher.fd == -1) + int sd; + char buf[BUFSIZ]; + ssize_t len; + + sd = inetd->watcher.fd; + if (sd == -1) return spawn_socket(inetd); - return -EEXIST; + /* Read anything lingering, or clean up socket after failure */ + len = recv(sd, buf, sizeof(buf), MSG_DONTWAIT); + _d("Read %d lingering bytes from socket before restarting %s ...", len, inetd->svc->cmd); + + /* Restore O_NONBLOCK for socket */ + fcntl(sd, F_SETFL, fcntl(sd, F_GETFL, 0) | O_NONBLOCK); + + _d("Re-starting %s socket watcher ...", inetd->svc->cmd); + uev_io_start(&inetd->watcher); + + return 0; } void inetd_stop(inetd_t *inetd) { if (inetd->watcher.fd != -1) { + _d("Stopping %s socket watcher ...", inetd->svc->cmd); uev_io_stop(&inetd->watcher); - shutdown(inetd->watcher.fd, SHUT_RDWR); - close(inetd->watcher.fd); - inetd->watcher.fd = -1; + + /* For dgram inetd services we block the parent SVC + * and halt the watcher, so don't close the socket! */ + if (inetd->svc->block != SVC_BLOCK_INETD_BUSY) { + shutdown(inetd->watcher.fd, SHUT_RDWR); + close(inetd->watcher.fd); + inetd->watcher.fd = -1; + } } } From 7f5f8ba09d756fce8c629de3ba918647ad828247 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 11:44:34 +0200 Subject: [PATCH 6/7] Minor grammar: "Connection" --> "connection" When starting an inetd service in verbose mode we print a progress message to the console appended with "Connection". This patch simply changes that to append with "connection" in lower case. Signed-off-by: Joachim Nilsson --- inetd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inetd.c b/inetd.c index 4d8e8767..2edab2ab 100644 --- a/inetd.c +++ b/inetd.c @@ -183,7 +183,7 @@ static void socket_cb(uev_t *w, void *arg, int UNUSED(events)) memcpy(task->username, svc->username, sizeof(task->username)); memcpy(task->group, svc->group, sizeof(task->group)); memcpy(task->args, svc->args, sizeof(task->args)); - snprintf(task->desc, sizeof(task->desc), "%s Connection", svc->desc); + snprintf(task->desc, sizeof(task->desc), "%s connection", svc->desc); task->stdin = stdin; service_step(task); From 5ea3cc0090b4e1717b9ab2b49c4d93eb75dbeb67 Mon Sep 17 00:00:00 2001 From: Joachim Nilsson Date: Sun, 24 Apr 2016 11:46:00 +0200 Subject: [PATCH 7/7] Reduce max backlog in listen(): 20 --> 10 Both the original MIT Athena inetd and the GNU inetutils inetd only keep a backlog of 10 connections in listen() for TCP sockets. This patch adjusts the Finit hard coded default accordingly. Signed-off-by: Joachim Nilsson --- inetd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inetd.c b/inetd.c index 2edab2ab..93138799 100644 --- a/inetd.c +++ b/inetd.c @@ -228,7 +228,7 @@ static int spawn_socket(inetd_t *inetd) if (inetd->port) { if (inetd->type == SOCK_STREAM) { - if (-1 == listen(sd, 20)) { + if (-1 == listen(sd, 10)) { FLOG_PERROR("Failed listening to inetd service %s", inetd->name); close(sd); return -errno;