diff --git a/deploy/files/usr/local/libexec/left4me/left4me-overlay b/deploy/files/usr/local/libexec/left4me/left4me-overlay index c01a5af..74ad954 100644 --- a/deploy/files/usr/local/libexec/left4me/left4me-overlay +++ b/deploy/files/usr/local/libexec/left4me/left4me-overlay @@ -55,6 +55,31 @@ def die(msg: str) -> None: sys.exit(1) +def _is_mountpoint( + path: str | Path, + mountinfo_path: str = "/proc/self/mountinfo", +) -> bool: + """Reliable mount-point check that handles same-fs bind mounts. + + `os.path.ismount()` compares `st_dev` of the path against its parent; + bind mounts on the same underlying filesystem share `st_dev` with their + parent, so `os.path.ismount()` returns False for them. The idmap binds + we install on `runtime//idmap/` are exactly that case. + Read /proc/self/mountinfo (field 5 is the mount point) for a check + that works regardless of mount type. The override is for tests only. + """ + abs_path = os.fspath(Path(path).resolve()) + try: + with open(mountinfo_path, "r", encoding="utf-8") as f: + for line in f: + fields = line.split() + if len(fields) >= 5 and fields[4] == abs_path: + return True + except OSError: + pass + return False + + def _lookup_uid(username: str) -> tuple[int, int]: """Return (uid, gid) for *username*, dying with a clear message if missing.""" try: @@ -256,7 +281,7 @@ def cmd_mount(name: str) -> None: idmap_dir.mkdir(mode=0o700, exist_ok=True) idmap_target.mkdir(mode=0o700, exist_ok=True) - if not os.path.ismount(idmap_target) or \ + if not _is_mountpoint(idmap_target) or \ os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") == "1": # --map-users / --map-groups argument format: # :: @@ -359,10 +384,11 @@ def cmd_umount(name: str) -> None: # Unwind idmap bind mounts, then remove the idmap directory. Each bind # is only umounted if it is still a mountpoint (idempotent across partial - # teardowns). + # teardowns). _is_mountpoint reads /proc/self/mountinfo because + # os.path.ismount misses same-fs bind mounts. for bind_umount_argv in bind_umount_argvs: target = Path(bind_umount_argv[-1]) - if os.path.ismount(target): + if _is_mountpoint(target): subprocess.run(bind_umount_argv, check=True) shutil.rmtree(idmap_dir, ignore_errors=True) diff --git a/l4d2host/tests/test_overlay_helper.py b/l4d2host/tests/test_overlay_helper.py index 6beda12..8044eb0 100644 --- a/l4d2host/tests/test_overlay_helper.py +++ b/l4d2host/tests/test_overlay_helper.py @@ -303,3 +303,47 @@ def test_rejects_upperdir_with_fuseoverlayfs_xattr(tmp_path: Path) -> None: result = _run(["mount", "alpha"], tmp_path) assert result.returncode != 0 assert "fuse-overlayfs xattr" in result.stderr + + +def _load_helper_module(): + """Import the helper script as a Python module for unit testing internals. + + The helper file has no .py extension, so importlib needs an explicit + SourceFileLoader rather than auto-detection. + """ + import importlib.util + from importlib.machinery import SourceFileLoader + loader = SourceFileLoader("left4me_overlay", str(HELPER_SOURCE)) + spec = importlib.util.spec_from_loader("left4me_overlay", loader) + assert spec is not None + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + return module + + +def test_is_mountpoint_detects_same_fs_bind_mount(tmp_path: Path) -> None: + """_is_mountpoint reads /proc/self/mountinfo so it works for same-fs bind mounts. + + Regression: os.path.ismount() compares st_dev against the parent, which + silently returns False for same-fs bind mounts. The idmap binds we install + on runtime//idmap/ are exactly that case, so an ismount-based + check skipped umount on stop and re-bound on top on start — accumulating + mount-table entries across stop/start cycles. + """ + helper = _load_helper_module() + + target = tmp_path / "some-bind" + target.mkdir() + abs_target = str(target.resolve()) + + mountinfo = tmp_path / "fake-mountinfo" + # mountinfo column 5 is the mountpoint; build a minimal line that exercises + # the parse without depending on the rest of the format. + mountinfo.write_text( + f"42 1 0:30 / {abs_target} rw,relatime - tmpfs tmpfs rw\n" + f"43 1 0:31 / /some/other/path rw,relatime - tmpfs tmpfs rw\n" + ) + + assert helper._is_mountpoint(target, str(mountinfo)) is True + assert helper._is_mountpoint(tmp_path / "not-a-mount", str(mountinfo)) is False + assert helper._is_mountpoint(target, str(tmp_path / "no-such-file")) is False