From bbe92ea5ccb6946441cbd862dcf4796a67218626 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 18 Oct 2023 19:23:42 +0800 Subject: [PATCH 1/4] core/namespace: merge if blocks --- src/core/namespace.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/core/namespace.c b/src/core/namespace.c index 7161ac6406..a9ea64be7a 100644 --- a/src/core/namespace.c +++ b/src/core/namespace.c @@ -2524,12 +2524,10 @@ int setup_namespace(const NamespaceParameters *p, char **error_path) { if (mount(NULL, "/", NULL, mount_propagation_flag | MS_REC, NULL) < 0) return log_debug_errno(errno, "Failed to remount '/' with desired mount flags: %m"); - /* bind_mount_in_namespace() will MS_MOVE into that directory, and that's only - * supported for non-shared mounts. This needs to happen after remounting / or it will fail. */ - if (setup_propagate) { - if (mount(NULL, p->incoming_dir, NULL, MS_SLAVE, NULL) < 0) - return log_debug_errno(errno, "Failed to remount %s with MS_SLAVE: %m", p->incoming_dir); - } + /* bind_mount_in_namespace() will MS_MOVE into that directory, and that's only supported for + * non-shared mounts. This needs to happen after remounting / or it will fail. */ + if (setup_propagate && mount(NULL, p->incoming_dir, NULL, MS_SLAVE, NULL) < 0) + return log_debug_errno(errno, "Failed to remount %s with MS_SLAVE: %m", p->incoming_dir); return 0; } From dbc0342e61aae84d29d52284949ae6070c6abc60 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 18 Oct 2023 21:22:19 +0800 Subject: [PATCH 2/4] core/exec-invoke: use correct exit status These calls can fail not only due to OOM. --- src/core/exec-invoke.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c index b6ab421a8d..c6ef2953c8 100644 --- a/src/core/exec-invoke.c +++ b/src/core/exec-invoke.c @@ -4524,7 +4524,7 @@ int exec_invoke( ngids_after_pam = getgroups_alloc(&gids_after_pam); if (ngids_after_pam < 0) { - *exit_status = EXIT_MEMORY; + *exit_status = EXIT_GROUP; return log_exec_error_errno(context, params, ngids_after_pam, "Failed to obtain groups after setting up PAM: %m"); } } @@ -4632,7 +4632,7 @@ int exec_invoke( ngids_after_pam, &gids_to_enforce); if (ngids_to_enforce < 0) { - *exit_status = EXIT_MEMORY; + *exit_status = EXIT_GROUP; return log_exec_error_errno(context, params, ngids_to_enforce, "Failed to merge group lists. Group membership might be incorrect: %m"); From f65a40fb0b92e0e487e4a94b125a3db6781f1f52 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 19 Oct 2023 20:23:54 +0800 Subject: [PATCH 3/4] cgroup-util: use RET_GATHER more, return first error --- src/basic/cgroup-util.c | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 74aee40f6a..24a59dd7f5 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -314,19 +314,17 @@ static int cg_kill_items( done = true; r = cg_enumerate_items(SYSTEMD_CGROUP_CONTROLLER, path, &f, item); - if (r < 0) { - if (ret >= 0 && r != -ENOENT) - return r; - - return ret; - } + if (r == -ENOENT) + break; + if (r < 0) + return RET_GATHER(ret, r); for (;;) { _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; r = cg_read_pidref(f, &pidref); if (r < 0) - return r; + return RET_GATHER(ret, r); if (r == 0) break; @@ -341,10 +339,9 @@ static int cg_kill_items( /* If we haven't killed this process yet, kill it */ r = pidref_kill(&pidref, sig); - if (r < 0) { - if (ret >= 0 && r != -ESRCH) - ret = r; - } else { + if (r < 0 && r != -ESRCH) + RET_GATHER(ret, r); + if (r >= 0) { if (flags & CGROUP_SIGCONT) (void) pidref_kill(&pidref, SIGCONT); @@ -359,17 +356,12 @@ static int cg_kill_items( done = false; r = set_put(s, PID_TO_PTR(pidref.pid)); - if (r < 0) { - if (ret >= 0) - return r; - - return ret; - } + if (r < 0) + return RET_GATHER(ret, r); } - /* To avoid racing against processes which fork - * quicker than we can kill them we repeat this until - * no new pids need to be killed. */ + /* To avoid racing against processes which fork quicker than we can kill them, we repeat this + * until no new pids need to be killed. */ } while (!done); From bca080531147e1452480863ad4d69cf4c040ef70 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 18 Oct 2023 23:29:00 +0800 Subject: [PATCH 4/4] core/service: check error first and log about errno Follow-up for becdfcb9f1cb555c50dcfe51894cb0b155f7f01e --- src/core/service.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index 02a35bb4ff..54090251a9 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -1074,10 +1074,10 @@ static int service_is_suitable_main_pid(Service *s, PidRef *pid, int prio) { return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(EPERM), "New main PID "PID_FMT" is the control process, refusing.", pid->pid); r = pidref_is_alive(pid); + if (r < 0) + return log_unit_full_errno(UNIT(s), prio, r, "Failed to check if main PID "PID_FMT" exists or is a zombie: %m", pid->pid); if (r == 0) return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(ESRCH), "New main PID "PID_FMT" does not exist or is a zombie.", pid->pid); - if (r < 0) - return log_unit_full_errno(UNIT(s), prio, r, "Failed to check if main PID "PID_FMT" exists or is a zombie.", pid->pid); owner = manager_get_unit_by_pidref(UNIT(s)->manager, pid); if (owner == UNIT(s)) {