Attachment 137469 Details for Bug 105165 – [3/3] Add a unit test for the dbus-daemon resetting its fd limit

[patch] [3/3] Add a unit test for the dbus-daemon resetting its fd limit

0003-Add-a-unit-test-for-the-dbus-daemon-resetting-its-fd.patch (text/plain), 11.75 KB, created by Simon McVittie on 2018-02-20 14:38:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2018-02-20 14:38:30 UTC

Size: 11.75 KB

patch

obsolete

>From a81a08a6e46ee8e9c21f8e2473507549e336830a Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Tue, 20 Feb 2018 11:45:39 +0000
>Subject: [PATCH 3/3] Add a unit test for the dbus-daemon resetting its fd
> limit
>
>Signed-off-by: Simon McVittie <smcv@collabora.com>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=105165
>---
> cmake/ConfigureChecks.cmake                        |   1 +
> cmake/config.h.cmake                               |   1 +
> configure.ac                                       |   3 +-
> test/Makefile.am                                   |   3 +
> .../valid-config-files/as-another-user.conf.in     |  28 +++++
> test/dbus-daemon.c                                 | 113 ++++++++++++++++++---
> tools/ci-build.sh                                  |   7 +-
> 7 files changed, 141 insertions(+), 15 deletions(-)
> create mode 100644 test/data/valid-config-files/as-another-user.conf.in
>
>diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake
>index 9a12bbbd..a9a5fc90 100644
>--- 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 e487acdf..b7b6ee84 100644
>--- a/cmake/config.h.cmake
>+++ b/cmake/config.h.cmake
>@@ -207,6 +207,7 @@
> #cmakedefine HAVE_DIRFD 1
> #cmakedefine HAVE_INOTIFY_INIT1 1
> #cmakedefine HAVE_GETRLIMIT 1
>+#cmakedefine HAVE_PRINTF 1
> #cmakedefine HAVE_SETRLIMIT 1
> #cmakedefine HAVE_UNIX_FD_PASSING 1
> 
>diff --git a/configure.ac b/configure.ac
>index 5bcfed22..f86753a0 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -612,7 +612,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
>@@ -687,6 +687,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 04bfc61c..5153ab6c 100644
>--- a/test/Makefile.am
>+++ b/test/Makefile.am
>@@ -440,6 +440,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 \
>@@ -557,6 +558,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),' \
>@@ -571,6 +573,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
>index 00000000..89d00c8c
>--- /dev/null
>+++ b/test/data/valid-config-files/as-another-user.conf.in
>@@ -0,0 +1,28 @@
>+
>+
>+<!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>
>+
>+  
>+  <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 7f990a7e..222b1028 100644
>--- 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)
>     {
>@@ -1843,6 +1853,71 @@ test_get_all (Fixture *f,
>   dbus_clear_message (&m);
> }
> 
>+#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)
>@@ -1910,55 +1985,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
> 
>@@ -2036,6 +2120,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 33d5ffe1..2876086c 100755
>--- a/tools/ci-build.sh
>+++ b/tools/ci-build.sh
>@@ -241,9 +241,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
>-- 
>2.16.1
>

Actions: View | Splinter Review

Attachments on bug 105165: 137441 | 137462 | 137464 | 137465 | 137466 | 137468 | 137469

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.