From: Yu Watanabe Date: Tue, 18 Feb 2025 19:46:08 +0000 (+0900) Subject: exec-invoke: add missing assertions and drop unnecessary conditions X-Git-Tag: v258-rc1~1198^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=059d23c966e285fcc05162001e66a31c28785f81;p=thirdparty%2Fsystemd.git exec-invoke: add missing assertions and drop unnecessary conditions Fixes CID#1534358. --- diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index ce334aca591..b2d1bd41a53 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -646,6 +646,7 @@ static int setup_confirm_stdio( _cleanup_close_ int fd = -EBADF, saved_stdin = -EBADF, saved_stdout = -EBADF; int r; + assert(context); assert(ret_saved_stdin); assert(ret_saved_stdout); @@ -1006,9 +1007,11 @@ static int enforce_user( const ExecContext *context, uid_t uid, uint64_t capability_ambient_set) { - assert(context); + int r; + assert(context); + if (!uid_is_valid(uid)) return 0; @@ -1434,6 +1437,8 @@ static bool context_has_syscall_logs(const ExecContext *c) { } static bool context_has_seccomp(const ExecContext *c) { + assert(c); + /* We need NNP if we have any form of seccomp and are unprivileged */ return c->lock_personality || c->memory_deny_write_execute || @@ -1495,7 +1500,10 @@ static bool seccomp_allows_drop_privileges(const ExecContext *c) { return !(has_capget || has_capset || has_prctl); } -static bool skip_seccomp_unavailable(const ExecContext *c, const ExecParameters *p, const char* msg) { +static bool skip_seccomp_unavailable(const ExecContext *c, const ExecParameters *p, const char *msg) { + assert(c); + assert(p); + assert(msg); if (is_seccomp_available()) return false; @@ -1794,6 +1802,7 @@ static int apply_protect_hostname(const ExecContext *c, const ExecParameters *p, assert(c); assert(p); + assert(ret_exit_status); if (c->protect_hostname == PROTECT_HOSTNAME_NO) return 0; @@ -1900,6 +1909,7 @@ static int build_environment( assert(c); assert(p); + assert(cgroup_context); assert(ret); #define N_ENV_VARS 20 @@ -2118,7 +2128,7 @@ static int build_environment( our_env[n_env++] = x; - if (cgroup_context && !path_equal(memory_pressure_path, "/dev/null")) { + if (!path_equal(memory_pressure_path, "/dev/null")) { _cleanup_free_ char *b = NULL, *e = NULL; if (asprintf(&b, "%s " USEC_FMT " " USEC_FMT, @@ -2159,6 +2169,9 @@ static int build_pass_environment(const ExecContext *c, char ***ret) { _cleanup_strv_free_ char **pass_env = NULL; size_t n_env = 0; + assert(c); + assert(ret); + STRV_FOREACH(i, c->pass_environment) { _cleanup_free_ char *x = NULL; char *v; @@ -2178,7 +2191,6 @@ static int build_pass_environment(const ExecContext *c, char ***ret) { } *ret = TAKE_PTR(pass_env); - return 0; } @@ -2383,7 +2395,7 @@ static int setup_private_users(PrivateUsers private_users, uid_t ouid, gid_t ogi return 1; } -static int can_mount_proc(const ExecContext *c, ExecParameters *p) { +static int can_mount_proc(const ExecContext *c, const ExecParameters *p) { _cleanup_close_pair_ int errno_pipe[2] = EBADF_PAIR; _cleanup_(sigkill_waitp) pid_t pid = 0; ssize_t n; @@ -2886,11 +2898,12 @@ fail: #if ENABLE_SMACK static int setup_smack( - const ExecParameters *params, const ExecContext *context, + const ExecParameters *params, int executable_fd) { int r; + assert(context); assert(params); assert(executable_fd >= 0); @@ -3158,13 +3171,14 @@ static int setup_ephemeral( int r; assert(context); + assert(runtime); assert(root_image); assert(root_directory); if (!*root_image && !*root_directory) return 0; - if (!runtime || !runtime->ephemeral_copy) + if (!runtime->ephemeral_copy) return 0; assert(runtime->ephemeral_storage_socket[0] >= 0); @@ -3376,6 +3390,8 @@ static int apply_mount_namespace( int r; assert(context); + assert(params); + assert(runtime); CLEANUP_ARRAY(bind_mounts, n_bind_mounts, bind_mount_free_many); @@ -3423,7 +3439,7 @@ static int apply_mount_namespace( * to world users. Inside of it there's a /tmp that is sticky, and that's the one we want to * use here. This does not apply when we are using /run/systemd/empty as fallback. */ - if (context->private_tmp == PRIVATE_TMP_CONNECTED && runtime && runtime->shared) { + if (context->private_tmp == PRIVATE_TMP_CONNECTED && runtime->shared) { if (streq_ptr(runtime->shared->tmp_dir, RUN_SYSTEMD_EMPTY)) tmp_dir = runtime->shared->tmp_dir; else if (runtime->shared->tmp_dir) @@ -3615,6 +3631,8 @@ static int apply_working_directory( int r; assert(context); + assert(params); + assert(runtime); if (context->working_directory_home) { /* Preferably use the data from $HOME, in case it was updated by a PAM module */ @@ -3635,7 +3653,7 @@ static int apply_working_directory( _cleanup_close_ int dfd = -EBADF; r = chase(wd, - (runtime ? runtime->ephemeral_copy : NULL) ?: context->root_directory, + runtime->ephemeral_copy ?: context->root_directory, CHASE_PREFIX_ROOT|CHASE_AT_RESOLVE_IN_ROOT, /* ret_path= */ NULL, &dfd); @@ -3653,11 +3671,13 @@ static int apply_root_directory( int *exit_status) { assert(context); + assert(params); + assert(runtime); assert(exit_status); if (params->flags & EXEC_APPLY_CHROOT) if (!needs_mount_ns && context->root_directory) - if (chroot((runtime ? runtime->ephemeral_copy : NULL) ?: context->root_directory) < 0) { + if (chroot(runtime->ephemeral_copy ?: context->root_directory) < 0) { *exit_status = EXIT_CHROOT; return -errno; } @@ -3668,7 +3688,8 @@ static int apply_root_directory( static int setup_keyring( const ExecContext *context, const ExecParameters *p, - uid_t uid, gid_t gid) { + uid_t uid, + gid_t gid) { key_serial_t keyring; int r = 0; @@ -3825,12 +3846,14 @@ static int close_remaining_fds( const ExecParameters *params, const ExecRuntime *runtime, int socket_fd, - const int *fds, size_t n_fds) { + const int *fds, + size_t n_fds) { size_t n_dont_close = 0; int dont_close[n_fds + 17]; assert(params); + assert(runtime); if (params->stdin_fd >= 0) dont_close[n_dont_close++] = params->stdin_fd; @@ -3846,15 +3869,14 @@ static int close_remaining_fds( n_dont_close += n_fds; } - if (runtime) - append_socket_pair(dont_close, &n_dont_close, runtime->ephemeral_storage_socket); + append_socket_pair(dont_close, &n_dont_close, runtime->ephemeral_storage_socket); - if (runtime && runtime->shared) { + if (runtime->shared) { append_socket_pair(dont_close, &n_dont_close, runtime->shared->netns_storage_socket); append_socket_pair(dont_close, &n_dont_close, runtime->shared->ipcns_storage_socket); } - if (runtime && runtime->dynamic_creds) { + if (runtime->dynamic_creds) { if (runtime->dynamic_creds->user) append_socket_pair(dont_close, &n_dont_close, runtime->dynamic_creds->user->storage_socket); if (runtime->dynamic_creds->group) @@ -4272,11 +4294,12 @@ static int setup_delegated_namespaces( assert(context); assert(params); + assert(runtime); assert(reterr_exit_status); if (exec_needs_network_namespace(context) && exec_namespace_is_delegated(context, params, CLONE_NEWNET) == delegate && - runtime && runtime->shared && runtime->shared->netns_storage_socket[0] >= 0) { + runtime->shared && runtime->shared->netns_storage_socket[0] >= 0) { /* Try to enable network namespacing if network namespacing is available and we have * CAP_NET_ADMIN in the current user namespace (either the system manager one or the unit's @@ -4303,7 +4326,7 @@ static int setup_delegated_namespaces( if (exec_needs_ipc_namespace(context) && exec_namespace_is_delegated(context, params, CLONE_NEWIPC) == delegate && - runtime && runtime->shared && runtime->shared->ipcns_storage_socket[0] >= 0) { + runtime->shared && runtime->shared->ipcns_storage_socket[0] >= 0) { if (ns_type_supported(NAMESPACE_IPC)) { r = setup_shareable_ns(runtime->shared->ipcns_storage_socket, CLONE_NEWIPC); @@ -4633,6 +4656,8 @@ int exec_invoke( assert(command); assert(context); assert(params); + assert(runtime); + assert(cgroup_context); assert(exit_status); /* This should be mostly redundant, as the log level is also passed as an argument of the executor, @@ -4793,7 +4818,7 @@ int exec_invoke( return log_exec_error_errno(context, params, errno, "Failed to update environment: %m"); } - if (context->dynamic_user && runtime && runtime->dynamic_creds) { + if (context->dynamic_user && runtime->dynamic_creds) { _cleanup_strv_free_ char **suggested_paths = NULL; /* On top of that, make sure we bypass our own NSS module nss-systemd comprehensively for any NSS @@ -4914,7 +4939,7 @@ int exec_invoke( } } - if (context->network_namespace_path && runtime && runtime->shared && runtime->shared->netns_storage_socket[0] >= 0) { + if (context->network_namespace_path && runtime->shared && runtime->shared->netns_storage_socket[0] >= 0) { r = open_shareable_ns_path(runtime->shared->netns_storage_socket, context->network_namespace_path, CLONE_NEWNET); if (r < 0) { *exit_status = EXIT_NETWORK; @@ -4922,7 +4947,7 @@ int exec_invoke( } } - if (context->ipc_namespace_path && runtime && runtime->shared && runtime->shared->ipcns_storage_socket[0] >= 0) { + if (context->ipc_namespace_path && runtime->shared && runtime->shared->ipcns_storage_socket[0] >= 0) { r = open_shareable_ns_path(runtime->shared->ipcns_storage_socket, context->ipc_namespace_path, CLONE_NEWIPC); if (r < 0) { *exit_status = EXIT_NAMESPACE; @@ -5143,7 +5168,7 @@ int exec_invoke( } } - if (cgroup_context && cg_unified() > 0 && is_pressure_supported() > 0) { + if (cg_unified() > 0 && is_pressure_supported() > 0) { if (cgroup_context_want_memory_pressure(cgroup_context)) { r = cg_get_path("memory", params->cgroup_path, "memory.pressure", &memory_pressure_path); if (r < 0) { @@ -5526,7 +5551,7 @@ int exec_invoke( /* LSM Smack needs the capability CAP_MAC_ADMIN to change the current execution security context of the * process. This is the latest place before dropping capabilities. Other MAC context are set later. */ if (use_smack) { - r = setup_smack(params, context, executable_fd); + r = setup_smack(context, params, executable_fd); if (r < 0 && !context->smack_process_label_ignore) { *exit_status = EXIT_SMACK_PROCESS_LABEL; return log_exec_error_errno(context, params, r, "Failed to set SMACK process label: %m");