From: Daan De Meyer Date: Fri, 15 Mar 2024 10:57:14 +0000 (+0100) Subject: Filter and sort all mounts in sandbox_cmd() X-Git-Tag: v23~90^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F2511%2Fhead;p=thirdparty%2Fmkosi.git Filter and sort all mounts in sandbox_cmd() We don't want users of sandbox_cmd() to have to care about mount ordering. Currently, if mounts with a more general destination are ordered after mounts with a more specific destination, the earlier mount is hidden by the later mount. By sorting by destination, we avoid this issue. --- diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py index 350240ca5..fdad185d6 100644 --- a/mkosi/sandbox.py +++ b/mkosi/sandbox.py @@ -96,6 +96,28 @@ def finalize_crypto_mounts(tools: Path = Path("/")) -> list[Mount]: ] +def finalize_mounts(mounts: Sequence[Mount]) -> list[PathString]: + mounts = list(set(mounts)) + + mounts = [ + m for m in mounts + if not any( + m != n and + m.devices == n.devices and + m.ro == n.ro and + m.required == n.required and + Path(m.src).is_relative_to(n.src) and + Path(m.dst).is_relative_to(n.dst) and + Path(m.src).relative_to(n.src) == Path(m.dst).relative_to(n.dst) + for n in mounts + ) + ] + + mounts = sorted(mounts, key=lambda m: (Path(m.dst), m.devices, not m.ro, m.required, Path(m.src))) + + return flatten(m.options() for m in mounts) + + def sandbox_cmd( *, network: bool = False, @@ -107,6 +129,7 @@ def sandbox_cmd( options: Sequence[PathString] = (), ) -> list[PathString]: cmdline: list[PathString] = [] + mounts = list(mounts) if not relaxed: # We want to use an empty subdirectory in the host's temporary directory as the sandbox's /var/tmp. To make @@ -126,10 +149,10 @@ def sandbox_cmd( # We mounted a subdirectory of TMPDIR to /var/tmp so we unset TMPDIR so that /tmp or /var/tmp are used instead. "--unsetenv", "TMPDIR", ] - allmounts = [Mount(tools / "usr", "/usr", ro=True)] + mounts += [Mount(tools / "usr", "/usr", ro=True)] if relaxed: - allmounts += [Mount("/tmp", "/tmp")] + mounts += [Mount("/tmp", "/tmp")] else: cmdline += [ "--tmpfs", "/tmp", @@ -137,10 +160,10 @@ def sandbox_cmd( ] if (tools / "nix/store").exists(): - allmounts += [Mount(tools / "nix/store", "/nix/store")] + mounts += [Mount(tools / "nix/store", "/nix/store")] if devices or relaxed: - allmounts += [ + mounts += [ Mount("/sys", "/sys"), Mount("/run", "/run"), Mount("/dev", "/dev", devices=True), @@ -153,7 +176,7 @@ def sandbox_cmd( for d in dirs: if Path(d).exists(): - allmounts += [Mount(d, d)] + mounts += [Mount(d, d)] if len(Path.cwd().parents) >= 2: # `Path.parents` only supports slices and negative indexing from Python 3.10 onwards. @@ -165,21 +188,20 @@ def sandbox_cmd( d = "" if d and d not in (*dirs, "/home", "/usr", "/nix", "/tmp"): - allmounts += [Mount(d, d)] + mounts += [Mount(d, d)] if vartmp: - allmounts += [Mount(vartmp, "/var/tmp")] + mounts += [Mount(vartmp, "/var/tmp")] for d in ("bin", "sbin", "lib", "lib32", "lib64"): if (p := tools / d).is_symlink(): cmdline += ["--symlink", p.readlink(), Path("/") / p.relative_to(tools)] elif p.is_dir(): - allmounts += [Mount(p, Path("/") / p.relative_to(tools), ro=True)] + mounts += [Mount(p, Path("/") / p.relative_to(tools), ro=True)] path = "/usr/bin:/usr/sbin" if tools != Path("/") else os.environ["PATH"] cmdline += ["--setenv", "PATH", f"/scripts:{path}", *options] - allmounts = [*allmounts, *mounts] if not relaxed: cmdline += ["--symlink", "../proc/self/mounts", "/etc/mtab"] @@ -190,15 +212,15 @@ def sandbox_cmd( # already exists on the host as otherwise we'd modify the host's /etc by creating the mountpoint ourselves (or # fail when trying to create it). if (tools / "etc/alternatives").exists() and (not relaxed or Path("/etc/alternatives").exists()): - allmounts += [Mount(tools / "etc/alternatives", "/etc/alternatives", ro=True)] + mounts += [Mount(tools / "etc/alternatives", "/etc/alternatives", ro=True)] if scripts: - allmounts += [Mount(scripts, "/scripts", ro=True)] + mounts += [Mount(scripts, "/scripts", ro=True)] if network and not relaxed and Path("/etc/resolv.conf").exists(): - allmounts += [Mount("/etc/resolv.conf", "/etc/resolv.conf")] + mounts += [Mount("/etc/resolv.conf", "/etc/resolv.conf")] - cmdline += flatten(mount.options() for mount in allmounts) + cmdline += finalize_mounts(mounts) # bubblewrap creates everything with a restricted mode so relax stuff as needed. ops = [] diff --git a/mkosi/tree.py b/mkosi/tree.py index ab4b80a19..1b6f65118 100644 --- a/mkosi/tree.py +++ b/mkosi/tree.py @@ -150,23 +150,18 @@ def rmtree(*paths: Path, tools: Path = Path("/"), sandbox: SandboxProtocol = nos return if find_binary("btrfs", root=tools) and (subvolumes := [p for p in paths if is_subvolume(p)]): - parents = sorted(set(p.parent for p in subvolumes)) - parents = [p for p in parents if all(p == q or not p.is_relative_to(q) for q in parents)] - # Silence and ignore failures since when not running as root, this will fail with a permission error unless the # btrfs filesystem is mounted with user_subvol_rm_allowed. run(["btrfs", "subvolume", "delete", *subvolumes], check=False, - sandbox=sandbox(mounts=[Mount(p, p) for p in parents]), + sandbox=sandbox(mounts=[Mount(p.parent, p.parent) for p in subvolumes]), stdout=subprocess.DEVNULL if not ARG_DEBUG.get() else None, stderr=subprocess.DEVNULL if not ARG_DEBUG.get() else None) paths = tuple(p for p in paths if p.exists()) if paths: - parents = sorted(set(p.parent for p in paths)) - parents = [p for p in parents if all(p == q or not p.is_relative_to(q) for q in parents)] run(["rm", "-rf", "--", *paths], - sandbox=sandbox(mounts=[Mount(p, p) for p in parents])) + sandbox=sandbox(mounts=[Mount(p.parent, p.parent) for p in paths])) def move_tree(