[shepherd] 10/10: shepherd: Add test ensuring proper use of close-on-exec.

[Ludovic Courtès]

civodul pushed a commit to branch master
in repository shepherd.

commit 978e5b41f439e4dca5692ac8c6355b4bdf7a830c
Author: Ludovic Courtès <ludo@gnu.org>
AuthorDate: Wed Sep 7 23:05:48 2022 +0200

    shepherd: Add test ensuring proper use of close-on-exec.
    
    * tests/close-on-exec.sh: New file.
    * Makefile.am (TESTS): Add it.
    (SHELL): Add CC.
    * modules/shepherd/service.scm (exec-command): Add comment.
---
 Makefile.am                  |   5 +-
 modules/shepherd/service.scm |   4 +
 tests/close-on-exec.sh       | 183 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b3a6b8e..81fb04d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -245,7 +245,8 @@ TESTS =                                             \
   tests/transient.sh                           \
   tests/inetd.sh                               \
   tests/systemd.sh                             \
-  tests/signals.sh
+  tests/signals.sh                             \
+  tests/close-on-exec.sh
 
 TEST_EXTENSIONS = .sh
 EXTRA_DIST += $(TESTS)
@@ -254,7 +255,7 @@ AM_TESTS_ENVIRONMENT =                              \
   unset XDG_CONFIG_HOME; unset LANGUAGE;       \
   LC_ALL=C LC_MESSAGES=C                       \
   PATH="$(abs_top_builddir):$$PATH"            \
-  SHELL="$(SHELL)" GUILE="$(GUILE)"            \
+  SHELL="$(SHELL)" GUILE="$(GUILE)" CC="$(CC)" \
   
GUILE_LOAD_PATH="$(abs_top_srcdir)/modules:$(abs_top_builddir)/modules:$$GUILE_LOAD_PATH"
 \
   
GUILE_LOAD_COMPILED_PATH="$(abs_top_srcdir)/modules:$(abs_top_builddir)/modules:$$GUILE_LOAD_COMPILED_PATH"
 
diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
index f45bbf5..dc5539d 100644
--- a/modules/shepherd/service.scm
+++ b/modules/shepherd/service.scm
@@ -1052,6 +1052,10 @@ false."
      ;; finalization thread since we will close its pipe, leading to
      ;; "error in the finalization thread: Bad file descriptor".
      (without-automatic-finalization
+      ;; TODO: Remove this loop.  Now that all internal file descriptors are
+      ;; close-on-exec, we can safely remove this loop, unless users cause
+      ;; shepherd to evaluate code that opens non-close-on-exec file
+      ;; descriptors.
       (let loop ((i (+ 3 (length extra-ports))))
         (when (< i max-fd)
           (catch-system-error (close-fdes i))
diff --git a/tests/close-on-exec.sh b/tests/close-on-exec.sh
new file mode 100644
index 0000000..d3fe8ee
--- /dev/null
+++ b/tests/close-on-exec.sh
@@ -0,0 +1,183 @@
+# GNU Shepherd --- Ensure file descriptors are not leaked to children.
+# Copyright © 2022 Ludovic Courtès <ludo@gnu.org>
+#
+# This file is part of the GNU Shepherd.
+#
+# The GNU Shepherd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or (at
+# your option) any later version.
+#
+# The GNU Shepherd is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with the GNU Shepherd.  If not, see <http://www.gnu.org/licenses/>.
+
+shepherd --version
+herd --version
+
+socket="t-socket-$$"
+conf="t-conf-$$"
+log="t-log-$$"
+pid="t-pid-$$"
+c_file="t-count-file-descriptors-$$.c"
+exe="$PWD/t-count-file-descriptors-$$"
+fd_count="$PWD/t-fd-count-$$"
+
+herd="herd -s $socket"
+
+trap "cat $log || true; rm -f $socket $conf $log $fd_count $c_file $exe;
+      test -f $pid && kill \`cat $pid\` || true; rm -f $pid" EXIT
+
+cat > "$c_file" <<EOF
+/* This program counts its own open file descriptors and writes
+   that number to $fd_count.  It's more reliable than using the
+   shell or Guile since those may open additional file descriptors.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <string.h>
+
+int
+main (int argc, char *argv[])
+{
+  DIR *dir;
+  struct dirent *ent;
+  size_t count;
+  FILE *log;
+
+  dir = opendir ("/proc/self/fd");
+  chdir ("/proc/self/fd");
+  for (count = 0, ent = NULL; ent = readdir (dir), ent != NULL; )
+    {
+      if (strcmp (ent->d_name, ".") == 0 || strcmp (ent->d_name, "..") == 0)
+        continue;
+
+      char target[1024];
+      ssize_t size;
+      size = readlink (ent->d_name, target, sizeof target);
+      target[size < 0 ? 0 : size] = '\0';
+      printf ("%s -> %s\n", ent->d_name, target);
+
+      count++;
+    }
+  closedir (dir);
+
+  log = fopen ("$fd_count", "w");
+  fprintf (log, "%zi\n", count);
+  fclose (log);
+
+  return EXIT_SUCCESS;
+}
+EOF
+
+"${CC:-gcc}" -Wall "$c_file" -o "$exe"
+"$exe"                         # try it out
+
+cat > "$conf" <<EOF
+(register-services
+ (make <service>
+   #:provides '(system-ctor)
+   #:start (make-system-constructor "$exe")
+   #:stop  (const #f)
+   #:one-shot? #t)
+ (make <service>
+   #:provides '(forkexec-ctor)
+   #:start (make-forkexec-constructor '("$(type -P sleep)" "100"))
+   #:stop (make-kill-destructor))
+ (make <service>
+   #:provides '(inetd-ctor)
+   #:start (make-inetd-constructor '("$exe")
+                                   (list
+                                    (endpoint (make-socket-address
+                                               AF_INET
+                                               INADDR_LOOPBACK
+                                               5555))))
+   #:stop  (make-inetd-destructor))
+ (make <service>
+   #:provides '(systemd-ctor)
+   #:start (make-systemd-constructor '("$exe")
+                                    (list
+                                     (endpoint (make-socket-address
+                                                AF_INET
+                                                INADDR_LOOPBACK
+                                                5556))))
+   #:stop  (make-systemd-destructor)))
+EOF
+
+rm -f "$pid" "$fd_count"
+shepherd -I -s "$socket" -c "$conf" -l "$log" --pid="$pid" &
+
+# Wait till it's ready.
+while ! test -f "$pid" ; do sleep 0.3 ; done
+
+shepherd_pid="`cat $pid`"
+kill -0 $shepherd_pid
+
+# Open listening sockets, which should all be SOCK_CLOEXEC.
+$herd start inetd-ctor
+
+ls -l /proc/$shepherd_pid/fd
+
+# Start 'system-ctor' and check how many open file descriptors it sees.
+$herd start system-ctor
+$herd status system-ctor
+while ! test -f "$fd_count" ; do sleep 0.3 ; done
+
+# The process running $exe must have seen the three standard file descriptors
+# plus an open descriptor on /proc/self/fd.  Note that $exe is executed by
+# 'system' so whether file descriptors are closed depends entirely on properly
+# mapping all of shepherd's internal-use file descriptors as O_CLOEXEC.
+test $(cat "$fd_count") -eq 4
+
+# Same test, this time with a process started with 'make-forkexec-constructor'
+# and thus 'exec-command'.
+$herd start forkexec-ctor
+$herd status forkexec-ctor
+
+pid="$($herd status forkexec-ctor | grep "Running value" \
+  | sed -e's/^.* \([0-9]\+\)\.$/\1/g')"
+kill -0 "$pid"
+
+ls -l "/proc/$pid/fd"
+test "$(cd "/proc/$pid/fd"; echo *)" = "0 1 2"
+$herd stop forkexec-ctor
+
+# Likewise for inetd and systemd services.
+
+connect_to_server ()
+{
+    rm -f "$fd_count"
+    guile -c "(use-modules (ice-9 match))
+      (define IN6ADDR_LOOPBACK 1)
+      (define address (make-socket-address AF_INET INADDR_LOOPBACK $1))
+      (define sock (socket (sockaddr:fam address) SOCK_STREAM 0))
+      (connect sock address)"
+    while ! test -f "$fd_count" ; do sleep 0.3 ; done
+}
+
+for i in $(seq 1 3)
+do
+    # Spawn the inetd service process by connecting to the endpoint.  It must
+    # have nothing but the 3 standard file descriptors open (plus one for
+    # /proc/self/fd).
+    connect_to_server 5555
+    test $(cat "$fd_count") -eq 4
+
+    # Spawn the systemd service by starting it (it actually immediately stops
+    # instead of calling 'accept', but it doesn't matter).  This one must have
+    # 4 open file descriptors, the 4th one being the socket.
+    $herd enable systemd-ctor
+    $herd start systemd-ctor
+    connect_to_server 5556
+    test $(cat "$fd_count") -eq 5
+    $herd stop systemd-ctor
+
+    $herd restart forkexec-ctor
+done