From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Wed, 2 Nov 2016 19:00:54 +0000 (-0400)
Subject: pid1: fix fd memleak when we hit FileDescriptorStoreMax limit
X-Git-Tag: v232~11^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F4456%2Fhead;p=thirdparty%2Fsystemd.git

pid1: fix fd memleak when we hit FileDescriptorStoreMax limit

Since service_add_fd_store() already does the check, remove the redundant check
from service_add_fd_store_set().

Also, print a warning when repopulating FDStore after daemon-reexec and we hit
the limit. This is a user visible issue, so we should not discard fds silently.
(Note that service_deserialize_item is impacted by the return value from
service_add_fd_store(), but we rely on the general error message, so the caller
does not need to be modified, and does not show up in the diff.)
---

diff --git a/src/core/service.c b/src/core/service.c
index 00ce500fd31..a7274a758fd 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -386,7 +386,9 @@ static int service_add_fd_store(Service *s, int fd, const char *name) {
         assert(fd >= 0);
 
         if (s->n_fd_store >= s->n_fd_store_max)
-                return 0;
+                return -EXFULL; /* Our store is full.
+                                 * Use this errno rather than E[NM]FILE to distinguish from
+                                 * the case where systemd itself hits the file limit. */
 
         LIST_FOREACH(fd_store, fs, s->fd_store) {
                 r = same_fd(fs->fd, fd);
@@ -430,10 +432,7 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name) {
 
         assert(s);
 
-        if (fdset_size(fds) <= 0)
-                return 0;
-
-        while (s->n_fd_store < s->n_fd_store_max) {
+        while (fdset_size(fds) > 0) {
                 _cleanup_close_ int fd = -1;
 
                 fd = fdset_steal_first(fds);
@@ -441,16 +440,17 @@ static int service_add_fd_store_set(Service *s, FDSet *fds, const char *name) {
                         break;
 
                 r = service_add_fd_store(s, fd, name);
+                if (r == -EXFULL)
+                        return log_unit_warning_errno(UNIT(s), r,
+                                                      "Cannot store more fds than FileDescriptorStoreMax=%u, closing remaining.",
+                                                      s->n_fd_store_max);
                 if (r < 0)
-                        return log_unit_error_errno(UNIT(s), r, "Couldn't add fd to fd store: %m");
+                        return log_unit_error_errno(UNIT(s), r, "Failed to add fd to store: %m");
                 if (r > 0)
                         log_unit_debug(UNIT(s), "Added fd %u (%s) to fd store.", fd, strna(name));
                 fd = -1;
         }
 
-        if (fdset_size(fds) > 0)
-                log_unit_warning(UNIT(s), "Tried to store more fds than FileDescriptorStoreMax=%u allows, closing remaining.", s->n_fd_store_max);
-
         return 0;
 }