fix(left4me-overlay): use /proc/self/mountinfo to detect bind mounts
os.path.ismount() compares st_dev against the parent dir, which silently
returns False for same-fs bind mounts. The idmap binds at runtime/<n>/
idmap/<basename> are exactly that case, so:
- cmd_umount skipped the bind-umount step every stop, leaving orphan
binds in PID 1's mount namespace.
- cmd_mount's idempotency check then "didn't see" the orphan and
re-bound on top, accumulating one mount per start/stop cycle.
Findmnt nesting like
/var/lib/left4me/runtime/2/idmap/overlays_9
└─/var/lib/left4me/runtime/2/idmap/overlays_9
is the visible symptom. Reboot wipes everything so the bug is invisible
on a fresh boot — only stop/start cycles accumulate.
Replace both ismount sites with a _is_mountpoint() helper that reads
/proc/self/mountinfo (column 5 is the mount point). Keep os.path.ismount
for the overlay merged check, where it's reliable (distinct fs type).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
2b20bffeb8
commit
dd918aca4b
2 changed files with 73 additions and 3 deletions
|
|
@ -55,6 +55,31 @@ def die(msg: str) -> None:
|
||||||
sys.exit(1)
|
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/<n>/idmap/<basename>` 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]:
|
def _lookup_uid(username: str) -> tuple[int, int]:
|
||||||
"""Return (uid, gid) for *username*, dying with a clear message if missing."""
|
"""Return (uid, gid) for *username*, dying with a clear message if missing."""
|
||||||
try:
|
try:
|
||||||
|
|
@ -256,7 +281,7 @@ def cmd_mount(name: str) -> None:
|
||||||
idmap_dir.mkdir(mode=0o700, exist_ok=True)
|
idmap_dir.mkdir(mode=0o700, exist_ok=True)
|
||||||
idmap_target.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":
|
os.environ.get("LEFT4ME_OVERLAY_PRINT_ONLY") == "1":
|
||||||
# --map-users / --map-groups argument format:
|
# --map-users / --map-groups argument format:
|
||||||
# <on-disk-uid>:<in-mount-uid>:<count>
|
# <on-disk-uid>:<in-mount-uid>:<count>
|
||||||
|
|
@ -359,10 +384,11 @@ def cmd_umount(name: str) -> None:
|
||||||
|
|
||||||
# Unwind idmap bind mounts, then remove the idmap directory. Each bind
|
# Unwind idmap bind mounts, then remove the idmap directory. Each bind
|
||||||
# is only umounted if it is still a mountpoint (idempotent across partial
|
# 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:
|
for bind_umount_argv in bind_umount_argvs:
|
||||||
target = Path(bind_umount_argv[-1])
|
target = Path(bind_umount_argv[-1])
|
||||||
if os.path.ismount(target):
|
if _is_mountpoint(target):
|
||||||
subprocess.run(bind_umount_argv, check=True)
|
subprocess.run(bind_umount_argv, check=True)
|
||||||
shutil.rmtree(idmap_dir, ignore_errors=True)
|
shutil.rmtree(idmap_dir, ignore_errors=True)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -303,3 +303,47 @@ def test_rejects_upperdir_with_fuseoverlayfs_xattr(tmp_path: Path) -> None:
|
||||||
result = _run(["mount", "alpha"], tmp_path)
|
result = _run(["mount", "alpha"], tmp_path)
|
||||||
assert result.returncode != 0
|
assert result.returncode != 0
|
||||||
assert "fuse-overlayfs xattr" in result.stderr
|
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/<n>/idmap/<basename> 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
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue