From 9b1419111aee0f4dca07593bd8e6db4ed94ce141 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 12 May 2017 11:32:53 +0200 Subject: [PATCH] core: only apply NonBlocking= to fds passed via socket activation Make sure to only apply the O_NONBLOCK flag to the fds passed via socket activation. Previously the flag was also applied to the fds which came from the fd store but this was incorrect since services, after being restarted, expect that these passed fds have their flags unchanged and can be reused as before. The documentation was a bit unclear about this so clarify it. --- man/systemd.service.xml | 17 ++++++++--------- src/core/execute.c | 21 ++++++++++++++------- src/core/execute.h | 1 + src/core/service.c | 14 ++++++++++---- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 9a0b72aca93..48c1ea15b17 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -782,15 +782,14 @@ NonBlocking= - Set the O_NONBLOCK flag - for all file descriptors passed via socket-based activation. - If true, all file descriptors >= 3 (i.e. all except stdin, - stdout, and stderr) will have the - O_NONBLOCK flag set and hence are in - non-blocking mode. This option is only useful in conjunction - with a socket unit, as described in - systemd.socket5. - Defaults to false. + Set the O_NONBLOCK flag for all file descriptors passed via socket-based + activation. If true, all file descriptors >= 3 (i.e. all except stdin, stdout, stderr), excluding those passed + in via the file descriptor storage logic (see FileDescriptorStoreMax= for details), will + have the O_NONBLOCK flag set and hence are in non-blocking mode. This option is only + useful in conjunction with a socket unit, as described in + systemd.socket5 and has no + effect on file descriptors which were previously saved in the file-descriptor store for example. Defaults to + false. diff --git a/src/core/execute.c b/src/core/execute.c index e97651105ee..7d9fc037b92 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -157,7 +157,7 @@ static int shift_fds(int fds[], unsigned n_fds) { return 0; } -static int flags_fds(const int fds[], unsigned n_fds, bool nonblock) { +static int flags_fds(const int fds[], unsigned n_fds, unsigned n_socket_fds, bool nonblock) { unsigned i; int r; @@ -166,13 +166,16 @@ static int flags_fds(const int fds[], unsigned n_fds, bool nonblock) { assert(fds); - /* Drops/Sets O_NONBLOCK and FD_CLOEXEC from the file flags */ + /* Drops/Sets O_NONBLOCK and FD_CLOEXEC from the file flags. + * O_NONBLOCK only applies to socket activation though. */ for (i = 0; i < n_fds; i++) { - r = fd_nonblock(fds[i], nonblock); - if (r < 0) - return r; + if (i < n_socket_fds) { + r = fd_nonblock(fds[i], nonblock); + if (r < 0) + return r; + } /* We unconditionally drop FD_CLOEXEC from the fds, * since after all we want to pass these fds to our @@ -2242,6 +2245,7 @@ static int exec_child( int socket_fd, int named_iofds[3], int *fds, unsigned n_fds, + unsigned n_socket_fds, char **files_env, int user_lookup_fd, int *exit_status, @@ -2669,7 +2673,7 @@ static int exec_child( if (r >= 0) r = shift_fds(fds, n_fds); if (r >= 0) - r = flags_fds(fds, n_fds, context->non_blocking); + r = flags_fds(fds, n_fds, n_socket_fds, context->non_blocking); if (r < 0) { *exit_status = EXIT_FDS; return r; @@ -2909,7 +2913,8 @@ int exec_spawn(Unit *unit, pid_t *ret) { _cleanup_strv_free_ char **files_env = NULL; - int *fds = NULL; unsigned n_fds = 0; + int *fds = NULL; + unsigned n_fds = 0, n_socket_fds = 0; _cleanup_free_ char *line = NULL; int socket_fd, r; int named_iofds[3] = { -1, -1, -1 }; @@ -2942,6 +2947,7 @@ int exec_spawn(Unit *unit, socket_fd = -1; fds = params->fds; n_fds = params->n_fds; + n_socket_fds = params->n_socket_fds; } r = exec_context_named_iofds(unit, context, params, named_iofds); @@ -2980,6 +2986,7 @@ int exec_spawn(Unit *unit, socket_fd, named_iofds, fds, n_fds, + n_socket_fds, files_env, unit->manager->user_lookup_fds[1], &exit_status, diff --git a/src/core/execute.h b/src/core/execute.h index 9f2b6fd39e3..dee7af14239 100644 --- a/src/core/execute.h +++ b/src/core/execute.h @@ -247,6 +247,7 @@ struct ExecParameters { int *fds; char **fd_names; unsigned n_fds; + unsigned n_socket_fds; ExecFlags flags; bool selinux_context_net:1; diff --git a/src/core/service.c b/src/core/service.c index b45929e5353..370a6316e7f 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1068,14 +1068,16 @@ static int service_coldplug(Unit *u) { return 0; } -static int service_collect_fds(Service *s, int **fds, char ***fd_names) { +static int service_collect_fds(Service *s, int **fds, char ***fd_names, unsigned *n_socket_fds) { _cleanup_strv_free_ char **rfd_names = NULL; _cleanup_free_ int *rfds = NULL; - int rn_fds = 0, r; + unsigned rn_socket_fds = 0; + int rn_fds = 0, r; assert(s); assert(fds); assert(fd_names); + assert(n_socket_fds); if (s->socket_fd >= 0) { @@ -1138,6 +1140,8 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names) { } } + rn_socket_fds = rn_fds; + if (s->n_fd_store > 0) { ServiceFDStore *fs; char **nl; @@ -1169,6 +1173,7 @@ static int service_collect_fds(Service *s, int **fds, char ***fd_names) { *fds = rfds; *fd_names = rfd_names; + *n_socket_fds = rn_socket_fds; rfds = NULL; rfd_names = NULL; @@ -1204,7 +1209,7 @@ static int service_spawn( _cleanup_strv_free_ char **final_env = NULL, **our_env = NULL, **fd_names = NULL; _cleanup_free_ int *fds = NULL; - unsigned n_fds = 0, n_env = 0; + unsigned n_fds = 0, n_socket_fds = 0, n_env = 0; const char *path; pid_t pid; @@ -1248,7 +1253,7 @@ static int service_spawn( s->exec_context.std_output == EXEC_OUTPUT_SOCKET || s->exec_context.std_error == EXEC_OUTPUT_SOCKET) { - r = service_collect_fds(s, &fds, &fd_names); + r = service_collect_fds(s, &fds, &fd_names, &n_socket_fds); if (r < 0) return r; @@ -1348,6 +1353,7 @@ static int service_spawn( exec_params.fds = fds; exec_params.fd_names = fd_names; exec_params.n_fds = n_fds; + exec_params.n_socket_fds = n_socket_fds; exec_params.confirm_spawn = manager_get_confirm_spawn(UNIT(s)->manager); exec_params.cgroup_supported = UNIT(s)->manager->cgroup_supported; exec_params.cgroup_path = path; -- 2.39.2