Wait until sandbox exec()'s specified command before continuing

Let's wait in spawn() until all setup logic has completed before
continuing if a sandbox is used. When using spawn() without run(),
this helps us fail early if the setup logic fails instead of later
on if we're not checking the result of the spawn() command immediately
afterwards.

As a side effect, this also acts as a synchronization point when using
systemd-run --scope where when spawn() returns, we can be 100% sure that
the scope has been created, which is important when calling RegisterMachine
from systemd-machined which does the wrong thing if the scope for the specified
machine does not yet exist and ends up killing the parent unit (often the user
session) instead of just the virtual machine.

diff --git a/mkosi/resources/man/mkosi-sandbox.1.md b/mkosi/resources/man/mkosi-sandbox.1.md
index b171e250a867b0689261266036b774c26bf91baf..e2103b5128d52031463839a014d276d8a36371b2 100644 (file)
--- a/mkosi/resources/man/mkosi-sandbox.1.md
+++ b/mkosi/resources/man/mkosi-sandbox.1.md
@@ -112,6 +112,11 @@ host system.
 `--unshare-ipc`
 :   Specifying this option makes `mkosi-sandbox` unshare an IPC namespace if possible.
 
+`--exec-fd FD`
+:   The specified `FD` will be closed when `mkosi-sandbox` calls `execvp()`. This is useful
+    to wait until all setup logic has completed before continuing execution in the parent
+    process invoking `mkosi-sandbox`.
+
 `--version`
 :   Show package version.
 

diff --git a/mkosi/run.py b/mkosi/run.py
index fb483319d82e55c365ecc4ae2049ca2ec5fa33e0..34d8d46875745448168f00eff6fb0a2b8b4d431a 100644 (file)
--- a/mkosi/run.py
+++ b/mkosi/run.py
@@ -167,6 +167,12 @@ def run(
     return CompletedProcess(cmdline, process.returncode, out, err)
 
 
+def fd_move_above(fd: int, above: int) -> int:
+    dup = fcntl.fcntl(fd, fcntl.F_DUPFD, above)
+    os.close(fd)
+    return dup
+
+
 def preexec(
     *,
     foreground: bool,
@@ -262,11 +268,23 @@ def spawn(
         prefix = [os.fspath(x) for x in sbx]
 
         if prefix:
-            prefix += ["--"]
+            prfd, pwfd = os.pipe2(os.O_CLOEXEC)
+
+            # Make sure the write end of the pipe (which we pass to the subprocess) is higher than all the
+            # file descriptors we'll pass to the subprocess, so that it doesn't accidentally get closed by
+            # the logic in preexec().
+            if pass_fds:
+                pwfd = fd_move_above(pwfd, list(pass_fds)[-1])
+
+            exec_prefix = ["--exec-fd", f"{SD_LISTEN_FDS_START + len(pass_fds)}", "--"]
+            pass_fds = [*pass_fds, pwfd]
+        else:
+            exec_prefix = []
+            prfd, pwfd = None, None
 
         try:
             with subprocess.Popen(
-                [*prefix, *cmdline],
+                [*prefix, *exec_prefix, *cmdline],
                 stdin=stdin,
                 stdout=stdout,
                 stderr=stderr,
@@ -285,15 +303,28 @@ def spawn(
                     pass_fds=pass_fds,
                 ),
             ) as proc:
+                if pwfd is not None:
+                    os.close(pwfd)
+
+                if prfd is not None:
+                    os.read(prfd, 1)
+                    os.close(prfd)
+
+                def failed() -> bool:
+                    return check and (rc := proc.poll()) is not None and rc not in success_exit_status
+
                 try:
-                    yield proc
+                    # Don't bother yielding if we've already failed by the time we get here. We'll raise an
+                    # exception later on so it's not a problem that we don't yield at all.
+                    if not failed():
+                        yield proc
                 except BaseException:
                     proc.terminate()
                     raise
                 finally:
                     returncode = proc.wait()
 
-                if check and returncode not in success_exit_status:
+                if failed():
                     if log:
                         log_process_failure(prefix, cmd, returncode)
                     if ARG_DEBUG_SHELL.get():

diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py
index 51ae6a3cf7c0e990a0a17719b974b7accd1e9690..83433720460951f90367e6bf46c50eec38db3ff3 100755 (executable)
--- a/mkosi/sandbox.py
+++ b/mkosi/sandbox.py
@@ -30,6 +30,9 @@ CLONE_NEWNS = 0x00020000
 CLONE_NEWUSER = 0x10000000
 EPERM = 1
 ENOENT = 2
+F_GETFD = 1
+F_SETFD = 2
+FD_CLOEXEC = 1
 LINUX_CAPABILITY_U32S_3 = 2
 LINUX_CAPABILITY_VERSION_3 = 0x20080522
 MNT_DETACH = 2
@@ -93,6 +96,7 @@ libc.pivot_root.argtypes = (ctypes.c_char_p, ctypes.c_char_p)
 libc.umount2.argtypes = (ctypes.c_char_p, ctypes.c_int)
 libc.capget.argtypes = (ctypes.c_void_p, ctypes.c_void_p)
 libc.capset.argtypes = (ctypes.c_void_p, ctypes.c_void_p)
+libc.fcntl.argtypes = (ctypes.c_int, ctypes.c_int, ctypes.c_int)
 
 
 def oserror(filename: str = "") -> None:
@@ -410,6 +414,11 @@ def is_relative_to(one: str, two: str) -> bool:
     return os.path.commonpath((one, two)) == two
 
 
+def fd_make_cloexec(fd: int) -> None:
+    flags = libc.fcntl(fd, F_GETFD, 0)
+    libc.fcntl(fd, F_SETFD, flags | FD_CLOEXEC)
+
+
 class FSOperation:
     def __init__(self, dst: str) -> None:
         self.dst = dst
@@ -694,6 +703,7 @@ mkosi-sandbox [OPTIONS...] COMMAND [ARGUMENTS...]
      --suppress-chown             Make chown() syscalls in the sandbox a noop
      --unshare-net                Unshare the network namespace if possible
      --unshare-ipc                Unshare the IPC namespace if possible
+     --exec-fd FD                 Close FD before execve()
 
 See the mkosi-sandbox(1) man page for details.\
 """
@@ -780,6 +790,8 @@ def main() -> None:
             unshare_net = True
         elif arg == "--unshare-ipc":
             unshare_ipc = True
+        elif arg == "--exec-fd":
+            fd_make_cloexec(int(argv.pop()))
         elif arg.startswith("-"):
             raise ValueError(f"Unrecognized option {arg}")
         else: