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.

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 9a0b72aca93b486ec2ef96ea6b9bb9a3b45d7c14..48c1ea15b17ca7bb7f55f6a84a11cbf7a8a3b9dc 100644 (file)
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -782,15 +782,14 @@
 
       <varlistentry>
         <term><varname>NonBlocking=</varname></term>
-        <listitem><para>Set the <constant>O_NONBLOCK</constant> 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
-        <constant>O_NONBLOCK</constant> flag set and hence are in
-        non-blocking mode. This option is only useful in conjunction
-        with a socket unit, as described in
-        <citerefentry><refentrytitle>systemd.socket</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
-        Defaults to false.</para></listitem>
+        <listitem><para>Set the <constant>O_NONBLOCK</constant> 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 <varname>FileDescriptorStoreMax=</varname> for details), will
+        have the <constant>O_NONBLOCK</constant> flag set and hence are in non-blocking mode. This option is only
+        useful in conjunction with a socket unit, as described in
+        <citerefentry><refentrytitle>systemd.socket</refentrytitle><manvolnum>5</manvolnum></citerefentry> and has no
+        effect on file descriptors which were previously saved in the file-descriptor store for example.  Defaults to
+        false.</para></listitem>
       </varlistentry>
 
       <varlistentry>

diff --git a/src/core/execute.c b/src/core/execute.c
index e97651105ee2f82067df361b21c49187aac09b58..7d9fc037b923d39c5c1cd56c17daa81afb6e04b1 100644 (file)
--- 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 9f2b6fd39e334111c3996ba43cc4449c963c1212..dee7af14239c74f554c7c9d867c25e405e8581f1 100644 (file)
--- 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 b45929e5353bd4018d52e82e6d41e0eb34f353fe..370a6316e7f8235f4d9a5b92c95fdfbd6738e30d 100644 (file)
--- 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;