From 988ca0953ea151624d32f5e3cd92334f7533ed11 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 20 Aug 2024 12:32:57 +0200 Subject: [PATCH 1/3] man: use standard-options for --no-ask-password everywhere --- man/hostnamectl.xml | 10 +--------- man/localectl.xml | 10 +--------- man/loginctl.xml | 8 +------- man/networkctl.xml | 12 +----------- man/resolvectl.xml | 11 +---------- man/run0.xml | 9 +-------- man/systemctl.xml | 21 +-------------------- man/systemd-run.xml | 10 +--------- man/timedatectl.xml | 10 +--------- 9 files changed, 9 insertions(+), 92 deletions(-) diff --git a/man/hostnamectl.xml b/man/hostnamectl.xml index 0f50bf9e44..70a6d295f6 100644 --- a/man/hostnamectl.xml +++ b/man/hostnamectl.xml @@ -172,15 +172,6 @@ The following options are understood: - - - - Do not query the user for authentication for - privileged operations. - - - - @@ -198,6 +189,7 @@ + diff --git a/man/localectl.xml b/man/localectl.xml index be0df8cc11..4ee21b0e11 100644 --- a/man/localectl.xml +++ b/man/localectl.xml @@ -177,15 +177,6 @@ - - - - Do not query the user for authentication for - privileged operations. - - - - @@ -201,6 +192,7 @@ + diff --git a/man/loginctl.xml b/man/loginctl.xml index 099de1dfe5..5a2eb3ba36 100644 --- a/man/loginctl.xml +++ b/man/loginctl.xml @@ -306,13 +306,6 @@ The following options are understood: - - - - Do not query the user for authentication for - privileged operations. - - @@ -410,6 +403,7 @@ + diff --git a/man/networkctl.xml b/man/networkctl.xml index 6cc6d0df1f..9c7e35b4b0 100644 --- a/man/networkctl.xml +++ b/man/networkctl.xml @@ -628,22 +628,12 @@ s - Service VLAN, m - Two-port MAC Relay (TPMR) + - - - - - - Do not query the user for authentication for privileged operations. - - - - - diff --git a/man/resolvectl.xml b/man/resolvectl.xml index 303aa93291..1cecd97631 100644 --- a/man/resolvectl.xml +++ b/man/resolvectl.xml @@ -495,16 +495,7 @@ - - - - - Do not query the user for authentication for privileged operations. - - - - - + diff --git a/man/run0.xml b/man/run0.xml index d7460d9d8c..16438b54cb 100644 --- a/man/run0.xml +++ b/man/run0.xml @@ -70,14 +70,6 @@ The following options are understood: - - - - Do not query the user for authentication for privileged operations. - - - - @@ -210,6 +202,7 @@ + diff --git a/man/systemctl.xml b/man/systemctl.xml index 1cf5eaf08e..8561e16944 100644 --- a/man/systemctl.xml +++ b/man/systemctl.xml @@ -2449,26 +2449,6 @@ Jan 12 10:46:45 example.com bluetoothd[8900]: gatt-time-server: Input/output err - - - - - When used with start and related - commands, disables asking for passwords. Background services - may require input of a password or passphrase string, for - example to unlock system hard disks or cryptographic - certificates. Unless this option is specified and the - command is invoked from a terminal, - systemctl will query the user on the - terminal for the necessary secrets. Use this option to - switch this behavior off. In this case, the password must be - supplied by some other means (for example graphical password - agents) or the service might fail. This also disables - querying the user for authentication for privileged - operations. - - - @@ -2896,6 +2876,7 @@ EOF + diff --git a/man/systemd-run.xml b/man/systemd-run.xml index 83c62afb59..77e61e6881 100644 --- a/man/systemd-run.xml +++ b/man/systemd-run.xml @@ -105,15 +105,6 @@ The following options are understood: - - - - Do not query the user for authentication for - privileged operations. - - - - @@ -528,6 +519,7 @@ + diff --git a/man/timedatectl.xml b/man/timedatectl.xml index ece8833295..11c3a57ec0 100644 --- a/man/timedatectl.xml +++ b/man/timedatectl.xml @@ -206,15 +206,6 @@ The following options are understood: - - - - Do not query the user for authentication for - privileged operations. - - - - @@ -285,6 +276,7 @@ + From df99a8ef3db84453fbe5acaa313ea7524b3ebb70 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 20 Aug 2024 22:01:09 +0200 Subject: [PATCH 2/3] process-util: check the flag instead of 'cgroup' param We might skip CLONE_INTO_CGROUP wholly if not supported. --- src/basic/process-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index a247b5a0b2..711457fa7b 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -2118,7 +2118,7 @@ int posix_spawn_wrapper( } if (ERRNO_IS_NOT_SUPPORTED(r)) { /* clone3() could also return EOPNOTSUPP if the target cgroup is in threaded mode. */ - if (cgroup && cg_is_threaded(cgroup) > 0) + if (FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP) && cg_is_threaded(cgroup) > 0) return -EUCLEAN; /* clone3() not available? */ From e06c5be29adbc38b3797c251fe78dbc6ebefa639 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Tue, 20 Aug 2024 20:04:46 +0200 Subject: [PATCH 3/3] process-util: always retry with pidfd_spawn() w/o cgroup first Follow-up for 7ac58157ca67ab001307f1fd72e0cc7c0c4e846a With the mentioned commit, iff E2BIG we'd retry pidfd_spawn() with POSIX_SPAWN_SETCGROUP disabled. However, the same strategy should actually apply to EOPNOTSUPP/ENOSYS/EPERM too - they can mean two things here: no clone3() or no CLONE_PIDFD. Therefore, let's first try clone() + CLONE_PIDFD, and fall further back to plain clone() (posix_spawn()) only as last resort. Plus, record the fact so that we don't unnecessarily retry every single time if CLONE_PIDFD is the one that's unavailable. --- src/basic/process-util.c | 72 +++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 35 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 711457fa7b..f30d9117a7 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -2059,10 +2059,14 @@ int posix_spawn_wrapper( _unused_ _cleanup_(posix_spawnattr_destroyp) posix_spawnattr_t *attr_destructor = &attr; #if HAVE_PIDFD_SPAWN - static bool setcgroup_supported = true; + static enum { + CLONE_ONLY_PID, + CLONE_CAN_PIDFD, /* 5.2 */ + CLONE_CAN_CGROUP, /* 5.7 */ + } clone_support = CLONE_CAN_CGROUP; _cleanup_close_ int cgroup_fd = -EBADF; - if (cgroup && setcgroup_supported) { + if (cgroup && clone_support >= CLONE_CAN_CGROUP) { _cleanup_free_ char *resolved_cgroup = NULL; r = cg_get_path_and_check( @@ -2093,45 +2097,43 @@ int posix_spawn_wrapper( return -r; #if HAVE_PIDFD_SPAWN - _cleanup_close_ int pidfd = -EBADF; + if (clone_support >= CLONE_CAN_PIDFD) { + _cleanup_close_ int pidfd = -EBADF; - r = pidfd_spawn(&pidfd, path, NULL, &attr, argv, envp); - if (r == E2BIG && FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP)) { - /* Some kernels (e.g., 5.4) support clone3 but they do not support CLONE_INTO_CGROUP. - * Retry pidfd_spawn() after removing the flag. */ - flags &= ~POSIX_SPAWN_SETCGROUP; - r = posix_spawnattr_setflags(&attr, flags); - if (r != 0) - return -r; r = pidfd_spawn(&pidfd, path, NULL, &attr, argv, envp); - /* if pidfd_spawn was successful after removing SPAWN_CGROUP, - * mark setcgroup_supported as false so that we do not retry every time */ - if (r == 0) - setcgroup_supported = false; - } - if (r == 0) { - r = pidref_set_pidfd_consume(ret_pidref, TAKE_FD(pidfd)); - if (r < 0) - return r; - - return FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP); - } - if (ERRNO_IS_NOT_SUPPORTED(r)) { - /* clone3() could also return EOPNOTSUPP if the target cgroup is in threaded mode. */ - if (FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP) && cg_is_threaded(cgroup) > 0) + if (ERRNO_IS_NOT_SUPPORTED(r) && FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP) && + cg_is_threaded(cgroup) > 0) /* clone3() could also return EOPNOTSUPP if the target cgroup is in threaded mode. */ return -EUCLEAN; + if ((ERRNO_IS_NOT_SUPPORTED(r) || ERRNO_IS_PRIVILEGE(r) || r == E2BIG) && + FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP)) { + /* Compiled on a newer host, or seccomp&friends blocking clone3()? Fallback, but + * need to disable POSIX_SPAWN_SETCGROUP, which is what redirects to clone3(). + * Note that we might get E2BIG here since some kernels (e.g. 5.4) support clone3() + * but not CLONE_INTO_CGROUP. */ - /* clone3() not available? */ - } else if (!ERRNO_IS_PRIVILEGE(r)) - return -r; + /* CLONE_INTO_CGROUP definitely won't work, hence remember the fact so that we don't + * retry every time. */ + assert(clone_support >= CLONE_CAN_CGROUP); + clone_support = CLONE_CAN_PIDFD; - /* Compiled on a newer host, or seccomp&friends blocking clone3()? Fallback, but need to change the - * flags to remove the cgroup one, which is what redirects to clone3() */ - if (FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP)) { - flags &= ~POSIX_SPAWN_SETCGROUP; - r = posix_spawnattr_setflags(&attr, flags); - if (r != 0) + flags &= ~POSIX_SPAWN_SETCGROUP; + r = posix_spawnattr_setflags(&attr, flags); + if (r != 0) + return -r; + + r = pidfd_spawn(&pidfd, path, NULL, &attr, argv, envp); + } + if (r == 0) { + r = pidref_set_pidfd_consume(ret_pidref, TAKE_FD(pidfd)); + if (r < 0) + return r; + + return FLAGS_SET(flags, POSIX_SPAWN_SETCGROUP); + } + if (!ERRNO_IS_NOT_SUPPORTED(r) && !ERRNO_IS_PRIVILEGE(r)) return -r; + + clone_support = CLONE_ONLY_PID; /* No CLONE_PIDFD either? */ } #endif