bus: Try to raise soft fd limit to match hard limit

Linux systems have traditionally set the soft limit to 1024 and the hard
limit to 4096. Recent versions of systemd keep the soft fd limit at
1024 to avoid breaking programs that still use select(), but raise the
hard limit to 512*1024, while in recent Debian versions a complicated
interaction between components gives a soft limit of 1024 and a hard
limit of 1024*1024. If we can, we might as well elevate our soft limit
to match the hard limit, minimizing the chance that we will run out of
file descriptor slots.

Unlike the previous code to raise the hard and soft limits to at least
65536, we do this even if we don't have privileges: privileges are
unnecessary to raise the soft limit up to the hard limit.

If we *do* have privileges, we also continue to raise the hard and soft
limits to at least 65536 if they weren't already that high, making
it harder to carry out a denial of service attack on the system bus on
systems that use the traditional limit (CVE-2014-7824).

As was previously the case on the system bus, we'll drop the limits back
to our initial limits before we execute a subprocess for traditional
(non-systemd) activation, if enabled.

systemd activation doesn't involve us starting subprocesses at all,
so in both cases activated services will still inherit the same limits
they did previously.

Reviewed-by: Lennart Poettering <lennart@poettering.net>
[smcv: Correct a comment based on Lennart's review, reword commit message]
Signed-off-by: Simon McVittie <smcv@collabora.com>

diff --git a/bus/bus.c b/bus/bus.c
index ca48b4bbc36b146a0e6c20ab6282c62ba518d4ea..3242e33f75417cd40dbbea49891ac9b678e029b1 100644 (file)
--- a/bus/bus.c
+++ b/bus/bus.c
@@ -716,11 +716,11 @@ raise_file_descriptor_limit (BusContext      *context)
   /* We used to compute a suitable rlimit based on the configured number
    * of connections, but that breaks down as soon as we allow fd-passing,
    * because each connection is allowed to pass 64 fds to us, and if
-   * they all did, we'd hit kernel limits. We now hard-code 64k as a
-   * good limit, like systemd does: that's enough to avoid DoS from
-   * anything short of multiple uids conspiring against us.
+   * they all did, we'd hit kernel limits. We now hard-code a good
+   * limit that is enough to avoid DoS from anything short of multiple
+   * uids conspiring against us, much like systemd does.
    */
-  if (!_dbus_rlimit_raise_fd_limit_if_privileged (65536, &error))
+  if (!_dbus_rlimit_raise_fd_limit (&error))
     {
       bus_context_log (context, DBUS_SYSTEM_LOG_WARNING,
                        "%s: %s", error.name, error.message);

diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index 2aee6c1c5f0335fe789088b48c1d9097fcb1c8a7..ccf3d9f5b46d885c6b50793afcc8abd45dbc95e4 100644 (file)
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -417,23 +417,15 @@ _dbus_rlimit_save_fd_limit (DBusError *error)
   return self;
 }
 
+/* Enough fds that we shouldn't run out, even if several uids work
+ * together to carry out a denial-of-service attack. This happens to be
+ * the same number that systemd < 234 would normally use. */
+#define ENOUGH_FDS 65536
+
 dbus_bool_t
-_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
-                                           DBusError    *error)
+_dbus_rlimit_raise_fd_limit (DBusError *error)
 {
-  struct rlimit lim;
-
-  /* No point to doing this practically speaking
-   * if we're not uid 0.  We expect the system
-   * bus to use this before we change UID, and
-   * the session bus takes the Linux default,
-   * currently 1024 for cur and 4096 for max.
-   */
-  if (getuid () != 0)
-    {
-      /* not an error, we're probably the session bus */
-      return TRUE;
-    }
+  struct rlimit old, lim;
 
   if (getrlimit (RLIMIT_NOFILE, &lim) < 0)
     {
@@ -442,22 +434,43 @@ _dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
       return FALSE;
     }
 
-  if (lim.rlim_cur == RLIM_INFINITY || lim.rlim_cur >= desired)
+  old = lim;
+
+  if (getuid () == 0)
     {
-      /* not an error, everything is fine */
-      return TRUE;
+      /* We are privileged, so raise the soft limit to at least
+       * ENOUGH_FDS, and the hard limit to at least the desired soft
+       * limit. This assumes we can exercise CAP_SYS_RESOURCE on Linux,
+       * or other OSs' equivalents. */
+      if (lim.rlim_cur != RLIM_INFINITY &&
+          lim.rlim_cur < ENOUGH_FDS)
+        lim.rlim_cur = ENOUGH_FDS;
+
+      if (lim.rlim_max != RLIM_INFINITY &&
+          lim.rlim_max < lim.rlim_cur)
+        lim.rlim_max = lim.rlim_cur;
     }
 
-  /* Ignore "maximum limit", assume we have the "superuser"
-   * privileges.  On Linux this is CAP_SYS_RESOURCE.
-   */
-  lim.rlim_cur = lim.rlim_max = desired;
+  /* Raise the soft limit to match the hard limit, which we can do even
+   * if we are unprivileged. In particular, systemd >= 240 will normally
+   * set rlim_cur to 1024 and rlim_max to 512*1024, recent Debian
+   * versions end up setting rlim_cur to 1024 and rlim_max to 1024*1024,
+   * and older and non-systemd Linux systems would typically set rlim_cur
+   * to 1024 and rlim_max to 4096. */
+  if (lim.rlim_max == RLIM_INFINITY || lim.rlim_cur < lim.rlim_max)
+    lim.rlim_cur = lim.rlim_max;
+
+  /* Early-return if there is nothing to do. */
+  if (lim.rlim_max == old.rlim_max &&
+      lim.rlim_cur == old.rlim_cur)
+    return TRUE;
 
   if (setrlimit (RLIMIT_NOFILE, &lim) < 0)
     {
       dbus_set_error (error, _dbus_error_from_errno (errno),
-                      "Failed to set fd limit to %u: %s",
-                      desired, _dbus_strerror (errno));
+                      "Failed to set fd limit to %lu: %s",
+                      (unsigned long) lim.rlim_cur,
+                      _dbus_strerror (errno));
       return FALSE;
     }
 
@@ -496,8 +509,7 @@ _dbus_rlimit_save_fd_limit (DBusError *error)
 }
 
 dbus_bool_t
-_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
-                                           DBusError    *error)
+_dbus_rlimit_raise_fd_limit (DBusError *error)
 {
   fd_limit_not_supported (error);
   return FALSE;

diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c
index ad41a44d0ced36f49f63af0ab077a8e2fdf10b57..639164ffe888cd711794928880e19eecc8fdf13a 100644 (file)
--- a/dbus/dbus-sysdeps-util-win.c
+++ b/dbus/dbus-sysdeps-util-win.c
@@ -273,8 +273,7 @@ _dbus_rlimit_save_fd_limit (DBusError *error)
 }
 
 dbus_bool_t
-_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int  desired,
-                                           DBusError    *error)
+_dbus_rlimit_raise_fd_limit (DBusError *error)
 {
   fd_limit_not_supported (error);
   return FALSE;

diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index b8d0391a7922a80f71e3d5d302dcffbd55a14af2..7c4204264a3b3604f31392730181e0658af784ce 100644 (file)
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -703,8 +703,7 @@ dbus_bool_t _dbus_replace_install_prefix (DBusString *path);
 typedef struct DBusRLimit DBusRLimit;
 
 DBusRLimit     *_dbus_rlimit_save_fd_limit                 (DBusError    *error);
-dbus_bool_t     _dbus_rlimit_raise_fd_limit_if_privileged  (unsigned int  desired,
-                                                            DBusError    *error);
+dbus_bool_t     _dbus_rlimit_raise_fd_limit                (DBusError    *error);
 dbus_bool_t     _dbus_rlimit_restore_fd_limit              (DBusRLimit   *saved,
                                                             DBusError    *error);
 void            _dbus_rlimit_free                          (DBusRLimit   *lim);