From: Daan De Meyer Date: Mon, 8 Jan 2024 22:31:37 +0000 (+0100) Subject: Simplify apivfs_cmd() and chroot_cmd() X-Git-Tag: v20~4^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e8adfc422fc2891d33eb9aa4848bb4afa7bc9c0a;p=thirdparty%2Fmkosi.git Simplify apivfs_cmd() and chroot_cmd() We move the setpgid logic to run(), avoiding the need to pass a tools argument to chroot_cmd() and apivfs_cmd(). We also try to remove as much logic from these functions as possible. Since we can't really assume that any logic we execute during the function will still hold true in the sandbox, so it's best to delay any logic execution until we're already in the sandbox (using the --ro-bind-try options of bubblewrap). We also rework the /etc/resolv.conf handling to simply make sure that /run/systemd/resolve exists in the chroot since if /etc/resolv.conf points to /run it'll almost certainly be to /run/systemd/resolv/stub-resolv.conf. --- diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 66a138ea4..924dc9f48 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -442,7 +442,6 @@ def run_prepare_scripts(context: Context, build: bool) -> None: chroot = chroot_cmd( context.root, resolve=True, - tools=context.config.tools(), options=[ "--bind", "/work", "/work", "--chdir", "/work/src", @@ -517,7 +516,6 @@ def run_build_scripts(context: Context) -> None: chroot = chroot_cmd( context.root, resolve=context.config.with_network, - tools=context.config.tools(), options=[ "--bind", "/work", "/work", "--chdir", "/work/src", @@ -587,7 +585,6 @@ def run_postinst_scripts(context: Context) -> None: chroot = chroot_cmd( context.root, resolve=context.config.with_network, - tools=context.config.tools(), options=[ "--bind", "/work", "/work", "--chdir", "/work/src", @@ -648,7 +645,6 @@ def run_finalize_scripts(context: Context) -> None: chroot = chroot_cmd( context.root, resolve=context.config.with_network, - tools=context.config.tools(), options=[ "--bind", "/work", "/work", "--chdir", "/work/src", diff --git a/mkosi/distributions/gentoo.py b/mkosi/distributions/gentoo.py index 1dbff224f..e6dfe95bc 100644 --- a/mkosi/distributions/gentoo.py +++ b/mkosi/distributions/gentoo.py @@ -26,7 +26,7 @@ from mkosi.util import sort_packages def invoke_emerge(context: Context, packages: Sequence[str] = (), apivfs: bool = True) -> None: run( - apivfs_cmd(context.root, tools=context.config.tools()) + [ + apivfs_cmd(context.root) + [ # We can't mount the stage 3 /usr using `options`, because bwrap isn't available in the stage 3 # tarball which is required by apivfs_cmd(), so we have to mount /usr from the tarball later # using another bwrap exec. @@ -161,7 +161,6 @@ class Installer(DistributionInstaller): chroot = chroot_cmd( stage3, - tools=context.config.tools(), options=["--bind", context.cache_dir / "repos", "/var/db/repos"], ) diff --git a/mkosi/installer/__init__.py b/mkosi/installer/__init__.py index 36afdab73..6135e03e0 100644 --- a/mkosi/installer/__init__.py +++ b/mkosi/installer/__init__.py @@ -41,12 +41,12 @@ def clean_package_manager_metadata(context: Context) -> None: def package_manager_scripts(context: Context) -> dict[str, list[PathString]]: return { - "pacman": apivfs_cmd(context.root, tools=context.config.tools()) + pacman_cmd(context), - "zypper": apivfs_cmd(context.root, tools=context.config.tools()) + zypper_cmd(context), - "dnf" : apivfs_cmd(context.root, tools=context.config.tools()) + dnf_cmd(context), - "rpm" : apivfs_cmd(context.root, tools=context.config.tools()) + rpm_cmd(context), + "pacman": apivfs_cmd(context.root) + pacman_cmd(context), + "zypper": apivfs_cmd(context.root) + zypper_cmd(context), + "dnf" : apivfs_cmd(context.root) + dnf_cmd(context), + "rpm" : apivfs_cmd(context.root) + rpm_cmd(context), } | { - command: apivfs_cmd(context.root, tools=context.config.tools()) + apt_cmd(context, command) for command in ( + command: apivfs_cmd(context.root) + apt_cmd(context, command) for command in ( "apt", "apt-cache", "apt-cdrom", diff --git a/mkosi/installer/apt.py b/mkosi/installer/apt.py index 594bdb6b0..0ccecebf0 100644 --- a/mkosi/installer/apt.py +++ b/mkosi/installer/apt.py @@ -113,7 +113,7 @@ def invoke_apt( *finalize_crypto_mounts(tools=context.config.tools()), *mounts, ], - ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else []) + ) + (apivfs_cmd(context.root) if apivfs else []) ), env=context.config.environment, ) diff --git a/mkosi/installer/dnf.py b/mkosi/installer/dnf.py index c410c7141..8a7c283f6 100644 --- a/mkosi/installer/dnf.py +++ b/mkosi/installer/dnf.py @@ -139,7 +139,7 @@ def invoke_dnf(context: Context, command: str, packages: Iterable[str], apivfs: context.cache_dir / "lib" / dnf_subdir(context), *finalize_crypto_mounts(tools=context.config.tools()), ], - ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else []) + ) + (apivfs_cmd(context.root) if apivfs else []) ), env=context.config.environment, ) diff --git a/mkosi/installer/pacman.py b/mkosi/installer/pacman.py index 1c3cfd905..993aeaebf 100644 --- a/mkosi/installer/pacman.py +++ b/mkosi/installer/pacman.py @@ -99,7 +99,7 @@ def invoke_pacman( "--bind", context.cache_dir / "cache/pacman/pkg", context.cache_dir / "cache/pacman/pkg", *finalize_crypto_mounts(tools=context.config.tools()), ], - ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else []) + ) + (apivfs_cmd(context.root) if apivfs else []) ), env=context.config.environment, ) diff --git a/mkosi/installer/zypper.py b/mkosi/installer/zypper.py index c800038f9..91e0caadd 100644 --- a/mkosi/installer/zypper.py +++ b/mkosi/installer/zypper.py @@ -87,7 +87,7 @@ def invoke_zypper( "--bind", context.cache_dir / "cache/zypp", context.cache_dir / "cache/zypp", *finalize_crypto_mounts(tools=context.config.tools()), ], - ) + (apivfs_cmd(context.root, tools=context.config.tools()) if apivfs else []) + ) + (apivfs_cmd(context.root) if apivfs else []) ), env=context.config.environment, ) diff --git a/mkosi/run.py b/mkosi/run.py index 54885d3f3..5fd7ef12f 100644 --- a/mkosi/run.py +++ b/mkosi/run.py @@ -289,6 +289,12 @@ def run( if preexec_fn: preexec_fn() + if ( + sandbox and + subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0 + ): + cmdline = ["setpgid", "--foreground", "--"] + cmdline + try: # subprocess.run() will use SIGKILL to kill processes when an exception is raised. # We'd prefer it to use SIGTERM instead but since this we can't configure which signal @@ -373,6 +379,13 @@ def spawn( if preexec_fn: preexec_fn() + if ( + foreground and + sandbox and + subprocess.run(sandbox + ["sh", "-c", "command -v setpgid"], stdout=subprocess.DEVNULL).returncode == 0 + ): + cmdline = ["setpgid", "--foreground", "--"] + cmdline + try: with subprocess.Popen( sandbox + cmdline, diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py index 1e730fdae..e574fd4b9 100644 --- a/mkosi/sandbox.py +++ b/mkosi/sandbox.py @@ -7,7 +7,6 @@ from collections.abc import Sequence from pathlib import Path from typing import Optional -from mkosi.run import find_binary from mkosi.types import PathString from mkosi.util import INVOKING_USER, flatten, one_zero @@ -33,18 +32,12 @@ def finalize_passwd_mounts(root: Path) -> list[PathString]: """ If passwd or a related file exists in the apivfs directory, bind mount it over the host files while we run the command, to make sure that the command we run uses user/group information from the apivfs - directory instead of from the host. If the file doesn't exist yet, mount over /dev/null instead. + directory instead of from the host. """ options: list[PathString] = [] for f in ("passwd", "group", "shadow", "gshadow"): - if not (Path("/etc") / f).exists(): - continue - p = root / "etc" / f - if p.exists(): - options += ["--bind", p, f"/etc/{f}"] - else: - options += ["--bind", "/dev/null", f"/etc/{f}"] + options += ["--ro-bind-try", root / "etc" / f, f"/etc/{f}"] return options @@ -161,14 +154,11 @@ def sandbox_cmd( cmdline += ["sh", "-c", f"{shm} && exec $0 \"$@\""] - if setpgid := find_binary("setpgid", root=tools): - cmdline += [setpgid, "--foreground", "--"] - return cmdline -def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]: - cmdline: list[PathString] = [ +def apivfs_cmd(root: Path) -> list[PathString]: + return [ "bwrap", "--dev-bind", "/", "/", "--tmpfs", root / "run", @@ -178,38 +168,30 @@ def apivfs_cmd(root: Path, *, tools: Path = Path("/")) -> list[PathString]: "--dev", root / "dev", # APIVFS generally means chrooting is going to happen so unset TMPDIR just to be safe. "--unsetenv", "TMPDIR", - ] - - if (root / "etc/machine-id").exists(): # Make sure /etc/machine-id is not overwritten by any package manager post install scripts. - cmdline += ["--ro-bind", root / "etc/machine-id", root / "etc/machine-id"] - - cmdline += finalize_passwd_mounts(root) - - if setpgid := find_binary("setpgid", root=tools): - cmdline += [setpgid, "--foreground", "--"] - - chmod = f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'}" - # Make sure anything running in the root directory thinks it's in a container. $container can't always be - # accessed so we write /run/host/container-manager as well which is always accessible. - container = f"mkdir {root}/run/host && echo mkosi >{root}/run/host/container-manager" - - cmdline += ["sh", "-c", f"{chmod} && {container} && exec $0 \"$@\""] - - return cmdline + "--ro-bind-try", root / "etc/machine-id", root / "etc/machine-id", + *finalize_passwd_mounts(root), + "sh", "-c", + f"chmod 1777 {root / 'tmp'} {root / 'var/tmp'} {root / 'dev/shm'} && " + f"chmod 755 {root / 'run'} && " + # Make sure anything running in the root directory thinks it's in a container. $container can't always be + # accessed so we write /run/host/container-manager as well which is always accessible. + f"mkdir -m 755 {root}/run/host && echo mkosi >{root}/run/host/container-manager && " + "exec $0 \"$@\"", + ] -def chroot_cmd( - root: Path, - *, - resolve: bool = False, - tools: Path = Path("/"), - options: Sequence[PathString] = (), -) -> list[PathString]: +def chroot_cmd(root: Path, *, resolve: bool = False, options: Sequence[PathString] = ()) -> list[PathString]: cmdline: list[PathString] = [ "sh", "-c", + f"trap 'rm -rf {root / 'work'}' EXIT && " + # /etc/resolv.conf can be a dangling symlink to /run/systemd/resolve/stub-resolv.conf. Bubblewrap tries to call + # mkdir() on each component of the path which means it will try to call + # mkdir(/run/systemd/resolve/stub-resolv.conf) which will fail unless /run/systemd/resolve exists already so + # we make sure that it already exists. + f"mkdir -p -m 755 {root / 'work'} {root / 'run/systemd'} {root / 'run/systemd/resolve'} && " # No exec here because we need to clean up the /work directory afterwards. - f"trap 'rm -rf {root / 'work'}' EXIT && mkdir -p {root / 'work'} && chown 777 {root / 'work'} && $0 \"$@\"", + f"$0 \"$@\"", "bwrap", "--dev-bind", root, "/", "--setenv", "container", "mkosi", @@ -218,20 +200,8 @@ def chroot_cmd( ] if resolve: - p = Path("etc/resolv.conf") - if (root / p).is_symlink(): - # For each component in the target path, bubblewrap will try to create it if it doesn't exist - # yet. If a component in the path is a dangling symlink, bubblewrap will end up calling - # mkdir(symlink) which obviously fails if multiple components of the dangling symlink path don't - # exist yet. As a workaround, we resolve the symlink ourselves so that bubblewrap will correctly - # create all missing components in the target path. - p = p.parent / (root / p).readlink() - - cmdline += ["--ro-bind", "/etc/resolv.conf", Path("/") / p] - - cmdline += [*options] + cmdline += ["--ro-bind-try", "/etc/resolv.conf", "/etc/resolv.conf"] - if setpgid := find_binary("setpgid", root=root): - cmdline += [setpgid, "--foreground", "--"] + cmdline += options - return apivfs_cmd(root, tools=tools) + cmdline + return apivfs_cmd(root) + cmdline