From: Daan De Meyer Date: Thu, 7 Dec 2023 10:21:30 +0000 (+0100) Subject: Run image builds in a fork again X-Git-Tag: v20~103^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4fecd1673f36bda467f33fc1575e066027f4a7ab;p=thirdparty%2Fmkosi.git Run image builds in a fork again This solves two problems: - When not using a tools tree, we can run qemu outside of the user namespace which means that we don't need to pass fds to /dev/kvm and /dev/vhost-vsock to keep things working unprivileged - The vmspawn verb we're about to introduce will not be able to run properly inside a user namespace, so we need to make sure we're not inside a user namespace after the image build. Compared to our original implementation of this way back with exception propagation, this time we opt to do things differently by doing all exception handling and logging inside the fork to avoid having to propagate exceptions. This makes the overall implementation a lot simpler. We can also run the other verbs outside of the user namespace as long as we're not using a tools tree. Because we want to keep support for using a tools tree with all verbs, we keep support for running them inside a user namespace as well. Because we already use INVOKING_USER everywhere, this actually turns out to require very little changes. We only need to make sure when starting virtiofsd that we unshare the user namespace ourselves if we're not uid mapping. --- diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 1ae70f872..a0deedcfa 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -56,6 +56,7 @@ from mkosi.run import ( bwrap, chroot_cmd, find_binary, + fork_and_wait, init_mount_namespace, run, ) @@ -2832,15 +2833,6 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None: if args.verb == Verb.qemu and d.feature(last) != ConfigFeature.disabled and d.available(log=True) } - # Get the user UID/GID either on the host or in the user namespace running the build - become_root() - init_mount_namespace() - - # For extra safety when running as root, remount a bunch of stuff read-only. - for d in ("/usr", "/etc", "/opt", "/srv", "/boot", "/efi", "/media", "/mnt"): - if Path(d).exists(): - run(["mount", "--rbind", d, d, "--options", "ro"]) - # First, process all directory removals because otherwise if different images share directories a later # image build could end up deleting the output generated by an earlier image build. @@ -2848,7 +2840,11 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None: if not needs_build(args, config) and args.verb != Verb.clean: continue - unlink_output(args, config) + def target() -> None: + become_root() + unlink_output(args, config) + + fork_and_wait(target) if args.verb == Verb.clean: return @@ -2861,29 +2857,40 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None: if not needs_build(args, config): continue - with ( - complete_step(f"Building {config.name()} image"), - mount_tools(config.tools_tree), - hide_host_directories(), - prepend_to_environ_path(config), - ): - # After tools have been mounted, check if we have what we need - check_tools(args, config) - - # Create these as the invoking user to make sure they're owned by the user running mkosi. - for p in ( - config.output_dir, - config.cache_dir, - config.build_dir, - config.workspace_dir, + def target() -> None: + become_root() + init_mount_namespace() + + # For extra safety when running as root, remount a bunch of stuff read-only. + for d in ("/usr", "/etc", "/opt", "/srv", "/boot", "/efi", "/media", "/mnt"): + if Path(d).exists(): + run(["mount", "--rbind", d, d, "--options", "ro"]) + + with ( + complete_step(f"Building {config.name()} image"), + mount_tools(config.tools_tree), + hide_host_directories(), + prepend_to_environ_path(config), ): - if p: - run(["mkdir", "--parents", p], user=INVOKING_USER.uid, group=INVOKING_USER.gid) + # After tools have been mounted, check if we have what we need + check_tools(args, config) + + # Create these as the invoking user to make sure they're owned by the user running mkosi. + for p in ( + config.output_dir, + config.cache_dir, + config.build_dir, + config.workspace_dir, + ): + if p: + run(["mkdir", "--parents", p], user=INVOKING_USER.uid, group=INVOKING_USER.gid) - with acl_toggle_build(config, INVOKING_USER.uid): - build_image(args, config) + with acl_toggle_build(config, INVOKING_USER.uid): + build_image(args, config) - build = True + fork_and_wait(target) + + build = True if build and args.auto_bump: bump_image_version() @@ -2891,11 +2898,17 @@ def run_verb(args: MkosiArgs, images: Sequence[MkosiConfig]) -> None: if args.verb == Verb.build: return - with ( - mount_usr(last.tools_tree), - mount_passwd(), - prepend_to_environ_path(last), - ): + if last.tools_tree: + become_root() + + with contextlib.ExitStack() as stack: + if os.getuid() == 0: + init_mount_namespace() + stack.enter_context(mount_usr(last.tools_tree)) + stack.enter_context(mount_passwd()) + + stack.enter_context(prepend_to_environ_path(last)) + check_tools(args, last) with prepend_to_environ_path(last): diff --git a/mkosi/__main__.py b/mkosi/__main__.py index 8239bd785..df2583543 100644 --- a/mkosi/__main__.py +++ b/mkosi/__main__.py @@ -1,47 +1,18 @@ # SPDX-License-Identifier: LGPL-2.1+ # PYTHON_ARGCOMPLETE_OK -import contextlib import faulthandler -import logging import shutil -import subprocess import sys -from collections.abc import Iterator from mkosi import run_verb from mkosi.config import parse_config -from mkosi.log import ARG_DEBUG, log_setup -from mkosi.run import ensure_exc_info, run +from mkosi.log import log_setup +from mkosi.run import run, uncaught_exception_handler from mkosi.util import INVOKING_USER -@contextlib.contextmanager -def propagate_failed_return() -> Iterator[None]: - try: - yield - except SystemExit as e: - if ARG_DEBUG.get(): - sys.excepthook(*ensure_exc_info()) - - sys.exit(e.code) - except KeyboardInterrupt: - if ARG_DEBUG.get(): - sys.excepthook(*ensure_exc_info()) - else: - logging.error("Interrupted") - - sys.exit(1) - except subprocess.CalledProcessError as e: - # Failures from qemu, ssh and systemd-nspawn are expected and we won't log stacktraces for those. - if ARG_DEBUG.get() and e.cmd and e.cmd[0] not in ("qemu", "ssh", "systemd-nspawn"): - sys.excepthook(*ensure_exc_info()) - - # We always log when subprocess.CalledProcessError is raised, so we don't log again here. - sys.exit(e.returncode) - - -@propagate_failed_return() +@uncaught_exception_handler(exit=sys.exit) def main() -> None: log_setup() # Ensure that the name and home of the user we are running as are resolved as early as possible. diff --git a/mkosi/qemu.py b/mkosi/qemu.py index b7b409b92..07d20a33f 100644 --- a/mkosi/qemu.py +++ b/mkosi/qemu.py @@ -33,7 +33,7 @@ from mkosi.config import ( ) from mkosi.log import die from mkosi.partition import finalize_root, find_partitions -from mkosi.run import MkosiAsyncioThread, find_binary, run, spawn +from mkosi.run import MkosiAsyncioThread, become_root, find_binary, run, spawn from mkosi.tree import copy_tree, rmtree from mkosi.types import PathString from mkosi.util import INVOKING_USER, StrEnum @@ -343,7 +343,8 @@ def start_virtiofsd(directory: Path, *, uidmap: bool) -> Iterator[Path]: cmdline, user=INVOKING_USER.uid if uidmap else None, group=INVOKING_USER.gid if uidmap else None, - pass_fds=(sock.fileno(),) + pass_fds=(sock.fileno(),), + preexec_fn=become_root if not uidmap and os.getuid() != 0 else None, ) as proc: try: yield path diff --git a/mkosi/run.py b/mkosi/run.py index a862c9e91..f8501acf1 100644 --- a/mkosi/run.py +++ b/mkosi/run.py @@ -21,7 +21,7 @@ import threading from collections.abc import Awaitable, Collection, Iterator, Mapping, Sequence from pathlib import Path from types import TracebackType -from typing import Any, Optional +from typing import Any, Callable, NoReturn, Optional from mkosi.log import ARG_DEBUG, ARG_DEBUG_SHELL, die from mkosi.types import _FILE, CompletedProcess, PathString, Popen @@ -161,6 +161,61 @@ def ensure_exc_info() -> tuple[type[BaseException], BaseException, TracebackType return (exctype, exc, tb) +@contextlib.contextmanager +def uncaught_exception_handler(exit: Callable[[int], NoReturn]) -> Iterator[None]: + rc = 0 + try: + yield + except SystemExit as e: + if ARG_DEBUG.get(): + sys.excepthook(*ensure_exc_info()) + + rc = e.code if isinstance(e.code, int) else 1 + except KeyboardInterrupt: + if ARG_DEBUG.get(): + sys.excepthook(*ensure_exc_info()) + else: + logging.error("Interrupted") + + rc = 1 + except subprocess.CalledProcessError as e: + # Failures from qemu, ssh and systemd-nspawn are expected and we won't log stacktraces for those. + # Failures from self come from the forks we spawn to build images in a user namespace. We've already done all + # the logging for those failures so we don't log stacktraces for those either. + if ARG_DEBUG.get() and e.cmd and e.cmd[0] not in ("self", "qemu", "ssh", "systemd-nspawn"): + sys.excepthook(*ensure_exc_info()) + + # We always log when subprocess.CalledProcessError is raised, so we don't log again here. + rc = e.returncode + except BaseException: + sys.excepthook(*ensure_exc_info()) + rc = 1 + finally: + sys.stdout.flush() + sys.stderr.flush() + exit(rc) + + +def fork_and_wait(target: Callable[[], None]) -> None: + pid = os.fork() + if pid == 0: + with uncaught_exception_handler(exit=os._exit): + make_foreground_process() + target() + + try: + _, status = os.waitpid(pid, 0) + except BaseException: + os.kill(pid, signal.SIGTERM) + _, status = os.waitpid(pid, 0) + finally: + make_foreground_process(new_process_group=False) + + rc = os.waitstatus_to_exitcode(status) + + if rc != 0: + raise subprocess.CalledProcessError(rc, ["self"]) + def run( cmdline: Sequence[PathString], check: bool = True, @@ -240,6 +295,7 @@ def spawn( env: Mapping[str, str] = {}, log: bool = True, foreground: bool = False, + preexec_fn: Optional[Callable[[], None]] = None, ) -> Iterator[Popen]: cmdline = [os.fspath(x) for x in cmdline] @@ -259,6 +315,12 @@ def spawn( **env, } + def preexec() -> None: + if foreground: + make_foreground_process() + if preexec_fn: + preexec_fn() + try: with subprocess.Popen( cmdline, @@ -270,7 +332,7 @@ def spawn( group=group, pass_fds=pass_fds, env=env, - preexec_fn=make_foreground_process if foreground else None, + preexec_fn=preexec, ) as proc: yield proc except FileNotFoundError as e: