From: Mike Yuan Date: Wed, 29 Oct 2025 21:13:35 +0000 (+0100) Subject: core/exec-invoke: store all stashed fds in ExecParameters, incl. OpenFile= ones X-Git-Tag: v259-rc1~217^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f70346fb87052f37d446d2e9ccf915e5289b2b87;p=thirdparty%2Fsystemd.git core/exec-invoke: store all stashed fds in ExecParameters, incl. OpenFile= ones Keeping a half-detached counter around brings nothing but confusion, and leads to fd leak in error paths. --- diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index fd84954e151..ce9ac0a00d3 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -1277,7 +1277,6 @@ static int setup_pam( uid_t uid, gid_t gid, char ***env, /* updated on success */ - const int fds[], size_t n_fds, bool needs_sandboxing, int exec_fd) { @@ -1307,7 +1306,6 @@ static int setup_pam( assert(user); assert(uid_is_valid(uid)); assert(gid_is_valid(gid)); - assert(fds || n_fds == 0); assert(env); /* We set up PAM in the parent process, then fork. The child will then stay around until killed via @@ -1393,7 +1391,7 @@ static int setup_pam( /* Make sure we don't keep open the passed fds in this child. We assume that otherwise only * those fds are open here that have been opened by PAM. */ - (void) close_many(fds, n_fds); + (void) close_many(params->fds, params->n_socket_fds + params->n_stashed_fds); /* Also close the 'exec_fd' in the child, since the service manager waits for the EOF induced * by the execve() to wait for completion, and if we'd keep the fd open here in the child @@ -1992,7 +1990,6 @@ static int build_environment( const ExecContext *c, const ExecParameters *p, const CGroupContext *cgroup_context, - size_t n_fds, const char *home, const char *username, const char *shell, @@ -2017,7 +2014,7 @@ static int build_environment( if (!our_env) return -ENOMEM; - if (n_fds > 0) { + if (p->n_socket_fds + p->n_stashed_fds > 0) { _cleanup_free_ char *joined = NULL; if (asprintf(&x, "LISTEN_PID="PID_FMT, getpid_cached()) < 0) @@ -2031,7 +2028,7 @@ static int build_environment( our_env[n_env++] = x; } - if (asprintf(&x, "LISTEN_FDS=%zu", n_fds) < 0) + if (asprintf(&x, "LISTEN_FDS=%zu", p->n_socket_fds + p->n_stashed_fds) < 0) return -ENOMEM; our_env[n_env++] = x; @@ -4426,9 +4423,8 @@ static int get_open_file_fd(const OpenFile *of) { return TAKE_FD(fd); } -static int collect_open_file_fds(ExecParameters *p, size_t *n_fds) { +static int collect_open_file_fds(ExecParameters *p) { assert(p); - assert(n_fds); LIST_FOREACH(open_files, of, p->open_files) { _cleanup_close_ int fd = -EBADF; @@ -4446,13 +4442,13 @@ static int collect_open_file_fds(ExecParameters *p, size_t *n_fds) { return log_error_errno(fd, "Failed to get OpenFile= file descriptor for '%s': %m", of->path); } - if (!GREEDY_REALLOC(p->fds, *n_fds + 1)) + if (!GREEDY_REALLOC(p->fds, p->n_stashed_fds + 1)) return log_oom(); if (strv_extend(&p->fd_names, of->fdname) < 0) return log_oom(); - p->fds[(*n_fds)++] = TAKE_FD(fd); + p->fds[p->n_stashed_fds++] = TAKE_FD(fd); } return 0; @@ -5028,8 +5024,6 @@ int exec_invoke( gid_t saved_gid = getgid(); uid_t uid = UID_INVALID; gid_t gid = GID_INVALID; - size_t n_fds, /* fds to pass to the child */ - n_keep_fds; /* total number of fds not to close */ int secure_bits; _cleanup_free_ gid_t *gids = NULL, *gids_after_pam = NULL; int ngids = 0, ngids_after_pam = 0; @@ -5070,7 +5064,6 @@ int exec_invoke( memmove(params->fd_names, params->fd_names + 1, params->n_stashed_fds * sizeof(char*)); params->fd_names[params->n_stashed_fds] = NULL; } - n_fds = params->n_socket_fds + params->n_stashed_fds; r = exec_context_named_iofds(context, params, named_iofds); if (r < 0) { @@ -5108,15 +5101,15 @@ int exec_invoke( /* In case anything used libc syslog(), close this here, too */ closelog(); - r = collect_open_file_fds(params, &n_fds); + r = collect_open_file_fds(params); if (r < 0) { *exit_status = EXIT_FDS; return log_error_errno(r, "Failed to get OpenFile= file descriptors: %m"); } - int keep_fds[n_fds + 4]; - memcpy_safe(keep_fds, params->fds, n_fds * sizeof(int)); - n_keep_fds = n_fds; + size_t n_keep_fds = params->n_socket_fds + params->n_stashed_fds; + int keep_fds[n_keep_fds + 4]; + memcpy_safe(keep_fds, params->fds, n_keep_fds * sizeof(int)); r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, ¶ms->exec_fd); if (r < 0) { @@ -5614,7 +5607,6 @@ int exec_invoke( context, params, cgroup_context, - n_fds, pwent_home, username, shell, @@ -5728,7 +5720,7 @@ int exec_invoke( /* All fds passed in the fds array will be closed in the pam child process. */ r = setup_pam(context, cgroup_context, params, username, uid, gid, &accum_env, - params->fds, n_fds, needs_sandboxing, params->exec_fd); + needs_sandboxing, params->exec_fd); if (r < 0) { *exit_status = EXIT_PAM; return log_error_errno(r, "Failed to set up PAM session: %m"); @@ -5962,9 +5954,10 @@ int exec_invoke( r = close_all_fds(keep_fds, n_keep_fds); if (r >= 0) - r = pack_fds(params->fds, n_fds); + r = pack_fds(params->fds, params->n_socket_fds + params->n_stashed_fds); if (r >= 0) - r = flag_fds(params->fds, params->n_socket_fds, n_fds, context->non_blocking); + r = flag_fds(params->fds, params->n_socket_fds, params->n_socket_fds + params->n_stashed_fds, + context->non_blocking); if (r < 0) { *exit_status = EXIT_FDS; return log_error_errno(r, "Failed to adjust passed file descriptors: %m");