From b09246352f751c95ab06f598b639f6f6ca0e67b1 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 2 Nov 2016 15:00:54 -0400 Subject: [PATCH] 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.) --- src/core/service.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 00ce500..a7274a7 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; } -- 2.7.4