core/exec-invoke: prevent potential double-close of exec_fd

If exec_fd is closed in add_shifted_fd() by close_and_replace(),
but something goes wrong later, we may close exec_fd twice
in exec_params_shallow_clear().

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index a5298329b0b38932276c717e957f069b28e7d898..ffb9db1d58cf4ef49ebf33d198bff1cf86fc10e4 100644 (file)
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -3601,32 +3601,29 @@ static int exec_context_cpu_affinity_from_numa(const ExecContext *c, CPUSet *ret
         return cpu_set_add_all(ret, &s);
 }
 
-static int add_shifted_fd(int *fds, size_t fds_size, size_t *n_fds, int fd, int *ret_fd) {
+static int add_shifted_fd(int *fds, size_t fds_size, size_t *n_fds, int *fd) {
         int r;
 
         assert(fds);
         assert(n_fds);
         assert(*n_fds < fds_size);
-        assert(ret_fd);
+        assert(fd);
 
-        if (fd < 0) {
-                *ret_fd = -EBADF;
-                return 0;
-        }
+        if (*fd < 0)
+               return 0;
 
-        if (fd < 3 + (int) *n_fds) {
+        if (*fd < 3 + (int) *n_fds) {
                 /* Let's move the fd up, so that it's outside of the fd range we will use to store
                  * the fds we pass to the process (or which are closed only during execve). */
 
-                r = fcntl(fd, F_DUPFD_CLOEXEC, 3 + (int) *n_fds);
+                r = fcntl(*fd, F_DUPFD_CLOEXEC, 3 + (int) *n_fds);
                 if (r < 0)
                         return -errno;
 
-                close_and_replace(fd, r);
+                close_and_replace(*fd, r);
         }
 
-        *ret_fd = fds[*n_fds] = fd;
-        (*n_fds) ++;
+        fds[(*n_fds)++] = *fd;
         return 1;
 }
 
@@ -3926,7 +3923,7 @@ int exec_invoke(
                 int *exit_status) {
 
         _cleanup_strv_free_ char **our_env = NULL, **pass_env = NULL, **joined_exec_search_path = NULL, **accum_env = NULL, **replaced_argv = NULL;
-        int r, ngids = 0, exec_fd;
+        int r, ngids = 0;
         _cleanup_free_ gid_t *supplementary_gids = NULL;
         const char *username = NULL, *groupname = NULL;
         _cleanup_free_ char *home_buffer = NULL, *memory_pressure_path = NULL;
@@ -4063,19 +4060,17 @@ int exec_invoke(
         memcpy_safe(keep_fds, fds, n_fds * sizeof(int));
         n_keep_fds = n_fds;
 
-        r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, params->exec_fd, &exec_fd);
+        r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &params->exec_fd);
         if (r < 0) {
                 *exit_status = EXIT_FDS;
-                return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
+                return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
         }
 
 #if HAVE_LIBBPF
-        if (params->bpf_outer_map_fd >= 0) {
-                r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, params->bpf_outer_map_fd, (int *)&params->bpf_outer_map_fd);
-                if (r < 0) {
-                        *exit_status = EXIT_FDS;
-                        return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
-                }
+        r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &params->bpf_outer_map_fd);
+        if (r < 0) {
+                *exit_status = EXIT_FDS;
+                return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
         }
 #endif
 
@@ -4756,10 +4751,10 @@ int exec_invoke(
                                              "EXECUTABLE=%s", command->path);
         }
 
-        r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, executable_fd, &executable_fd);
+        r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &executable_fd);
         if (r < 0) {
                 *exit_status = EXIT_FDS;
-                return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
+                return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
         }
 
 #if HAVE_SELINUX
@@ -5207,13 +5202,13 @@ int exec_invoke(
 
         log_command_line(context, params, "Executing", executable, final_argv);
 
-        if (exec_fd >= 0) {
+        if (params->exec_fd >= 0) {
                 uint8_t hot = 1;
 
                 /* We have finished with all our initializations. Let's now let the manager know that. From this point
                  * on, if the manager sees POLLHUP on the exec_fd, then execve() was successful. */
 
-                if (write(exec_fd, &hot, sizeof(hot)) < 0) {
+                if (write(params->exec_fd, &hot, sizeof(hot)) < 0) {
                         *exit_status = EXIT_EXEC;
                         return log_exec_error_errno(context, params, errno, "Failed to enable exec_fd: %m");
                 }
@@ -5221,13 +5216,13 @@ int exec_invoke(
 
         r = fexecve_or_execve(executable_fd, executable, final_argv, accum_env);
 
-        if (exec_fd >= 0) {
+        if (params->exec_fd >= 0) {
                 uint8_t hot = 0;
 
                 /* The execve() failed. This means the exec_fd is still open. Which means we need to tell the manager
                  * that POLLHUP on it no longer means execve() succeeded. */
 
-                if (write(exec_fd, &hot, sizeof(hot)) < 0) {
+                if (write(params->exec_fd, &hot, sizeof(hot)) < 0) {
                         *exit_status = EXIT_EXEC;
                         return log_exec_error_errno(context, params, errno, "Failed to disable exec_fd: %m");
                 }