Add a unit test for the dbus-daemon resetting its fd limit

Reviewed-by: David King <dking@redhat.com>
[smcv: Fix typo in cmake macro name]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
(cherry picked from commit 49ca421997d91d3e01626b2c92a826e6a5db0b2f)

diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake
index 9a12bbb..a9a5fc9 100644 (file)
--- a/cmake/ConfigureChecks.cmake
+++ b/cmake/ConfigureChecks.cmake
@@ -63,6 +63,7 @@ check_symbol_exists(SCM_RIGHTS    "sys/types.h;sys/socket.h;sys/un.h" HAVE_UNIX_
 check_symbol_exists(prctl        "sys/prctl.h"              HAVE_PRCTL)
 check_symbol_exists(raise        "signal.h"                 HAVE_RAISE)
 check_symbol_exists(getrlimit    "sys/resource.h;sys/time.h" HAVE_GETRLIMIT)
+check_symbol_exists(prlimit      "sys/resource.h;sys/time.h" HAVE_PRLIMIT)
 check_symbol_exists(setrlimit    "sys/resource.h;sys/time.h" HAVE_SETRLIMIT)
 
 check_struct_member(cmsgcred cmcred_pid "sys/types.h sys/socket.h" HAVE_CMSGCRED)   #  dbus-sysdeps.c

diff --git a/cmake/config.h.cmake b/cmake/config.h.cmake
index 2a89810..1119182 100644 (file)
--- a/cmake/config.h.cmake
+++ b/cmake/config.h.cmake
@@ -206,6 +206,7 @@
 #cmakedefine HAVE_DIRFD 1
 #cmakedefine HAVE_INOTIFY_INIT1 1
 #cmakedefine HAVE_GETRLIMIT 1
+#cmakedefine HAVE_PRLIMIT 1
 #cmakedefine HAVE_SETRLIMIT 1
 #cmakedefine HAVE_UNIX_FD_PASSING 1
 

diff --git a/configure.ac b/configure.ac
index 70fb030..5b0416d 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -615,7 +615,7 @@ AC_DEFINE_UNQUOTED([DBUS_USE_SYNC], [$have_sync], [Use the gcc __sync extension]
 AC_SEARCH_LIBS(socket,[socket network])
 AC_CHECK_FUNC(gethostbyname,,[AC_CHECK_LIB(nsl,gethostbyname)])
 
-AC_CHECK_FUNCS([vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull issetugid getresuid setresuid getrlimit])
+AC_CHECK_FUNCS([vsnprintf vasprintf nanosleep usleep setenv clearenv unsetenv socketpair getgrouplist fpathconf setrlimit poll setlocale localeconv strtoll strtoull issetugid getresuid setresuid getrlimit prlimit])
 
 AC_CHECK_HEADERS([syslog.h])
 if test "x$ac_cv_header_syslog_h" = "xyes"; then
@@ -690,6 +690,7 @@ closedir(dirp);
 fi
 
 AC_CHECK_HEADERS(sys/resource.h)
+AC_CHECK_HEADERS([sys/time.h])
 
 AC_CHECK_HEADERS(dirent.h)
 

diff --git a/test/Makefile.am b/test/Makefile.am
index 63b130c..6a6e1a3 100644 (file)
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -433,6 +433,7 @@ in_data = \
        data/systemd-activation/com.example.SystemdActivatable3.service.in \
        data/valid-config-files-system/debug-allow-all-fail.conf.in \
        data/valid-config-files-system/debug-allow-all-pass.conf.in \
+       data/valid-config-files/as-another-user.conf.in \
        data/valid-config-files/count-fds.conf.in \
        data/valid-config-files/debug-allow-all-sha1.conf.in \
        data/valid-config-files/debug-allow-all.conf.in \
@@ -576,6 +577,7 @@ uninstalled-config-local:
                sed \
                        -e 's,[@]DBUS_TEST_DATA[@],@abs_builddir@/data,' \
                        -e 's,[@]DBUS_TEST_EXEC[@],@abs_builddir@,' \
+                       -e 's,[@]DBUS_USER[@],$(DBUS_USER),' \
                        -e 's,[@]EXEEXT[@],$(EXEEXT),' \
                        -e 's,[@]TEST_LAUNCH_HELPER_BINARY[@],@abs_top_builddir@/bus/dbus-daemon-launch-helper-test$(EXEEXT),' \
                        -e 's,[@]TEST_LISTEN[@],$(TEST_LISTEN),' \
@@ -590,6 +592,7 @@ if DBUS_ENABLE_INSTALLED_TESTS
                sed \
                        -e 's,[@]DBUS_TEST_DATA[@],$(testexecdir)/data,' \
                        -e 's,[@]DBUS_TEST_EXEC[@],$(testexecdir),' \
+                       -e 's,[@]DBUS_USER[@],$(DBUS_USER),' \
                        -e 's,[@]EXEEXT[@],$(EXEEXT),' \
                        -e 's,[@]TEST_LAUNCH_HELPER_BINARY[@],/bin/false,' \
                        -e 's,[@]TEST_LISTEN[@],$(TEST_LISTEN),' \

diff --git a/test/data/valid-config-files/as-another-user.conf.in b/test/data/valid-config-files/as-another-user.conf.in
new file mode 100644 (file)
index 0000000..89d00c8
--- /dev/null
+++ b/test/data/valid-config-files/as-another-user.conf.in
@@ -0,0 +1,28 @@
+<!--
+Configuration similar to the system bus, to be used when testing rlimit
+handling.
+-->
+
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-Bus Bus Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+  <type>system</type>
+  <user>@DBUS_USER@</user>
+  <auth>EXTERNAL</auth>
+
+  <!-- This is not the same as the real system bus: we don't want to collide -->
+  <listen>@TEST_LISTEN@</listen>
+
+  <policy context="default">
+    <allow user="*"/>
+    <deny own="*"/>
+    <deny send_type="method_call"/>
+    <allow send_type="signal"/>
+    <allow send_requested_reply="true" send_type="method_return"/>
+    <allow send_requested_reply="true" send_type="error"/>
+    <allow receive_type="method_call"/>
+    <allow receive_type="method_return"/>
+    <allow receive_type="error"/>
+    <allow receive_type="signal"/>
+  </policy>
+</busconfig>

diff --git a/test/dbus-daemon.c b/test/dbus-daemon.c
index a783b59..311cc08 100644 (file)
--- a/test/dbus-daemon.c
+++ b/test/dbus-daemon.c
@@ -42,6 +42,7 @@
 #include <string.h>
 
 #ifdef DBUS_UNIX
+# include <pwd.h>
 # include <unistd.h>
 # include <sys/types.h>
 
@@ -49,6 +50,14 @@
     /* The CMake build system doesn't know how to check for this yet */
 #   include <gio/gunixfdmessage.h>
 # endif
+
+# ifdef HAVE_SYS_RESOURCE_H
+#   include <sys/resource.h>
+# endif
+
+# ifdef HAVE_SYS_TIME_H
+#   include <sys/time.h>
+# endif
 #endif
 
 /* Platforms where we know that credentials-passing passes both the
@@ -149,6 +158,7 @@ typedef struct {
     const char *bug_ref;
     guint min_messages;
     const char *config_file;
+    TestUser user;
     enum { SPECIFY_ADDRESS = 0, RELY_ON_DEFAULT } connect_mode;
 } Config;
 
@@ -178,8 +188,8 @@ setup (Fixture *f,
     }
 
   f->address = test_get_dbus_daemon (config ? config->config_file : NULL,
-                                     TEST_USER_ME, NULL,
-                                     &f->daemon_pid);
+                                     config ? config->user : TEST_USER_ME,
+                                     NULL, &f->daemon_pid);
 
   if (f->address == NULL)
     {
@@ -2017,6 +2027,71 @@ test_get_all (Fixture *f,
   dbus_clear_pending_call (&pc);
 }
 
+#define DESIRED_RLIMIT 65536
+
+#ifdef DBUS_UNIX
+static void
+test_fd_limit (Fixture *f,
+               gconstpointer context)
+{
+#ifdef HAVE_PRLIMIT
+  struct rlimit lim;
+  const struct passwd *pwd = NULL;
+#endif
+
+  if (f->skip)
+    return;
+
+#ifdef HAVE_PRLIMIT
+
+  if (getuid () != 0)
+    {
+      g_test_skip ("Cannot test, only uid 0 is expected to raise fd limit");
+      return;
+    }
+
+  pwd = getpwnam (DBUS_USER);
+
+  if (pwd == NULL)
+    {
+      gchar *message = g_strdup_printf ("user '%s' does not exist",
+          DBUS_USER);
+
+      g_test_skip (message);
+      g_free (message);
+      return;
+    }
+
+  if (prlimit (getpid (), RLIMIT_NOFILE, NULL, &lim) < 0)
+    g_error ("prlimit(): %s", g_strerror (errno));
+
+  g_test_message ("our RLIMIT_NOFILE: rlim_cur: %ld, rlim_max: %ld",
+                  (long) lim.rlim_cur, (long) lim.rlim_max);
+
+  if (lim.rlim_cur == RLIM_INFINITY || lim.rlim_cur >= DESIRED_RLIMIT)
+    {
+      /* The dbus-daemon will have inherited our large rlimit */
+      g_test_skip ("Cannot test, our own fd limit was already large");
+      return;
+    }
+
+  if (prlimit (f->daemon_pid, RLIMIT_NOFILE, NULL, &lim) < 0)
+    g_error ("prlimit(): %s", g_strerror (errno));
+
+  g_test_message ("dbus-daemon's RLIMIT_NOFILE: rlim_cur: %ld, rlim_max: %ld",
+                  (long) lim.rlim_cur, (long) lim.rlim_max);
+
+  if (lim.rlim_cur != RLIM_INFINITY)
+    g_assert_cmpint (lim.rlim_cur, >=, DESIRED_RLIMIT);
+
+#else /* !HAVE_PRLIMIT */
+
+  g_test_skip ("prlimit() not supported on this platform");
+
+#endif /* !HAVE_PRLIMIT */
+}
+#endif
+
 static void
 teardown (Fixture *f,
     gconstpointer context G_GNUC_UNUSED)
@@ -2084,55 +2159,64 @@ teardown (Fixture *f,
 
 static Config limited_config = {
     "34393", 10000, "valid-config-files/incoming-limit.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config finite_timeout_config = {
     NULL, 1, "valid-config-files/finite-timeout.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 #ifdef DBUS_UNIX
 static Config listen_unix_runtime_config = {
     "61303", 1, "valid-config-files/listen-unix-runtime.conf",
-    RELY_ON_DEFAULT
+    TEST_USER_ME, RELY_ON_DEFAULT
 };
 #endif
 
 static Config max_completed_connections_config = {
     NULL, 1, "valid-config-files/max-completed-connections.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config max_connections_per_user_config = {
     NULL, 1, "valid-config-files/max-connections-per-user.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config max_replies_per_connection_config = {
     NULL, 1, "valid-config-files/max-replies-per-connection.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config max_match_rules_per_connection_config = {
     NULL, 1, "valid-config-files/max-match-rules-per-connection.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config max_names_per_connection_config = {
     NULL, 1, "valid-config-files/max-names-per-connection.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 #if defined(DBUS_UNIX) && defined(HAVE_UNIX_FD_PASSING) && defined(HAVE_GIO_UNIX)
 static Config pending_fd_timeout_config = {
     NULL, 1, "valid-config-files/pending-fd-timeout.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
 };
 
 static Config count_fds_config = {
     NULL, 1, "valid-config-files/count-fds.conf",
-    SPECIFY_ADDRESS
+    TEST_USER_ME, SPECIFY_ADDRESS
+};
+#endif
+
+#if defined(DBUS_UNIX)
+static Config as_another_user_config = {
+    NULL, 1, "valid-config-files/as-another-user.conf",
+    /* We start the dbus-daemon as root and drop privileges, like the
+     * real system bus does */
+    TEST_USER_ROOT, SPECIFY_ADDRESS
 };
 #endif
 
@@ -2210,6 +2294,11 @@ main (int argc,
    * and that blocks on a round-trip to the dbus-daemon */
   g_test_add ("/unix-runtime-is-default", Fixture, &listen_unix_runtime_config,
       setup, test_echo, teardown);
+
+  g_test_add ("/fd-limit/session", Fixture, NULL,
+              setup, test_fd_limit, teardown);
+  g_test_add ("/fd-limit/system", Fixture, &as_another_user_config,
+              setup, test_fd_limit, teardown);
 #endif
 
   return g_test_run ();

diff --git a/tools/ci-build.sh b/tools/ci-build.sh
index d0938ee..88c0571 100755 (executable)
--- a/tools/ci-build.sh
+++ b/tools/ci-build.sh
@@ -239,9 +239,12 @@ case "$ci_buildsys" in
             gnome-desktop-testing-runner -d /usr/local/share dbus/ || \
                 maybe_fail_tests
 
-            # these tests benefit from being re-run as root
+            # these tests benefit from being re-run as root, and one
+            # test needs a finite fd limit to be useful
             sudo env LD_LIBRARY_PATH=/usr/local/lib \
-            gnome-desktop-testing-runner -d /usr/local/share \
+            bash -c 'ulimit -S -n 1024; ulimit -H -n 4096; exec "$@"' bash \
+                gnome-desktop-testing-runner -d /usr/local/share \
+                dbus/test-dbus-daemon_with_config.test \
                 dbus/test-uid-permissions_with_config.test || \
                 maybe_fail_tests
         fi