diff --git a/src/borg/archive.py b/src/borg/archive.py index e5884efc6..7bcf0a4ec 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -35,7 +35,7 @@ from .helpers import HardLinkManager from .helpers import archive_hostname, archive_username from .helpers import ChunkIteratorFileWrapper, open_item from .helpers import Error, IntegrityError, set_ec, sig_int -from .platform import uid2user, user2uid, gid2group, group2gid, get_birthtime_ns +from .platform import uid2user, user2uid, gid2group, group2gid, get_birthtime_ns, on_different_mounts from .helpers import parse_timestamp, archive_ts_now from .helpers import OutputTimestamp, format_timedelta, format_file_size, file_status, FileSize from .helpers import ArchiveFormatter @@ -349,6 +349,27 @@ def stat_update_check(st_old, st_curr): return st_curr +def is_automount(st_old, st_curr, parent_fd, fd): + """ + Check whether opening the directory triggered an automount, see #6652. + + stat() does not trigger an automount (e.g. autofs, systemd automount units or the ZFS + snapshot directories in .zfs/snapshot), but opening the directory does. st_old is then the + stat of the automount point and st_curr (the fstat() of fd) is the stat of the root directory + of the filesystem that got mounted there, so they look like a different inode. + + We only accept this if both are directories and if fd (the opened directory) is on a + different mount than parent_fd (its parent directory). + """ + if parent_fd is None or fd is None: + return False + if not (stat.S_ISDIR(st_old.st_mode) and stat.S_ISDIR(st_curr.st_mode)): + return False + if st_old.st_ino == st_curr.st_ino and st_old.st_dev == st_curr.st_dev: + return False + return on_different_mounts(parent_fd, fd) is True + + @contextmanager def OsOpen(*, flags, path=None, parent_fd=None, name=None, noatime=False, op="open"): with backup_io(op): diff --git a/src/borg/archiver/create_cmd.py b/src/borg/archiver/create_cmd.py index e7610c081..b1112dce2 100644 --- a/src/borg/archiver/create_cmd.py +++ b/src/borg/archiver/create_cmd.py @@ -11,7 +11,7 @@ from io import TextIOWrapper from ._common import with_repository, Highlander from .. import helpers from ..archive import Archive, Statistics, is_special, SF_DATALESS -from ..archive import BackupError, BackupOSError, BackupItemExcluded, backup_io, OsOpen, stat_update_check +from ..archive import BackupError, BackupOSError, BackupItemExcluded, backup_io, OsOpen, stat_update_check, is_automount from ..archive import FilesystemObjectProcessors, MetadataCollector, ChunksProcessor from ..cache import Cache from ..constants import * # NOQA @@ -55,11 +55,31 @@ def stat_root(path): """ st = os_stat(path=path, parent_fd=None, name=None, follow_symlinks=False) if not stat.S_ISLNK(st.st_mode): - return st, False + return stat_dir_mounted(path, st), False try: - return os_stat(path=path, parent_fd=None, name=None, follow_symlinks=True), True + st = os_stat(path=path, parent_fd=None, name=None, follow_symlinks=True) except FileNotFoundError: raise BackupBrokenSymlinkError("stat", "broken symlink, skipping it") from None + return stat_dir_mounted(path, st, follow_symlinks=True), True + + +def stat_dir_mounted(path, st, follow_symlinks=False): + """ + If st (the stat of path) is a directory, open it and return its fstat, otherwise return st. + + stat() does not trigger an automount, but opening the directory does, so we get the stat + of the root directory of the filesystem mounted there (not of the automount point), see #6652. + """ + if not stat.S_ISDIR(st.st_mode): + return st + flags = flags_dir_follow if follow_symlinks else flags_dir + try: + with OsOpen(path=path, flags=flags, noatime=True, op="dir_open") as fd: + # fd is None for directories on windows. + return st if fd is None else os.fstat(fd) + except (BackupOSError, OSError): + # the caller opens the directory again later and handles the error there. + return st class CreateMixIn: @@ -668,7 +688,17 @@ class CreateMixIn: # child_fd is None for directories on windows, in that case a race condition check is not possible. if child_fd is not None: with backup_io("fstat"): - st = stat_update_check(st, os.fstat(child_fd)) + st_curr = os.fstat(child_fd) + automounted = is_automount(st, st_curr, parent_fd, child_fd) + if automounted: + # opening the directory mounted a filesystem there, see #6652. + # redo the decisions we made based on the stat of the automount point. + st = st_curr + if (st.st_ino, st.st_dev) in skip_inodes: + return + recurse = restrict_dev is None or st.st_dev == restrict_dev + else: + st = stat_update_check(st, st_curr) if recurse: tag_names = dir_is_tagged(path, exclude_caches, exclude_if_present, dir_fd=child_fd) if tag_names: @@ -932,6 +962,9 @@ class CreateMixIn: macOS examples are the apfs mounts of a typical macOS installation. Therefore, when using ``--one-file-system``, you should double-check that the backup works as intended. + Automount points (e.g. autofs, systemd automount units, ZFS ``.zfs/snapshot/*``) get mounted when + borg opens them. For these, borg uses the device number of the filesystem that got mounted there. + .. _list_item_flags: Item flags diff --git a/src/borg/platform/__init__.py b/src/borg/platform/__init__.py index cdfbd6bb8..cfa5b56a9 100644 --- a/src/borg/platform/__init__.py +++ b/src/borg/platform/__init__.py @@ -25,6 +25,7 @@ if is_linux: # pragma: linux only from .linux import acl_get, acl_set from .linux import acl_text_to_xattr # type: ignore[no-redef] from .linux import set_flags, get_flags + from .linux import on_different_mounts from .linux import SyncFile from .posix import process_alive, local_pid_alive from .posix import get_errno @@ -34,6 +35,7 @@ elif is_freebsd: # pragma: freebsd only from .freebsd import listxattr, getxattr, setxattr from .freebsd import acl_get, acl_set from .freebsd import set_flags + from .base import on_different_mounts from .base import get_flags from .base import SyncFile from .posix import process_alive, local_pid_alive @@ -44,6 +46,7 @@ elif is_netbsd: # pragma: netbsd only from .netbsd import listxattr, getxattr, setxattr from .base import acl_get, acl_set from .base import set_flags, get_flags + from .base import on_different_mounts from .base import SyncFile from .posix import process_alive, local_pid_alive from .posix import get_errno @@ -54,6 +57,7 @@ elif is_darwin: # pragma: darwin only from .darwin import acl_get, acl_set from .darwin import is_darwin_feature_64_bit_inode, _get_birthtime_ns from .darwin import set_flags + from .base import on_different_mounts from .darwin import fdatasync, sync_dir # type: ignore[no-redef] from .base import get_flags from .base import SyncFile @@ -65,6 +69,7 @@ elif is_sunos: # pragma: sunos only from .solaris import listxattr, getxattr, setxattr from .base import acl_get, acl_set from .base import set_flags, get_flags + from .base import on_different_mounts from .base import SyncFile from .posix import process_alive, local_pid_alive from .posix import get_errno @@ -75,6 +80,7 @@ elif not is_win32: # pragma: posix only from .base import listxattr, getxattr, setxattr from .base import acl_get, acl_set from .base import set_flags, get_flags + from .base import on_different_mounts from .base import SyncFile from .posix import process_alive, local_pid_alive from .posix import get_errno @@ -85,6 +91,7 @@ else: # pragma: win32 only from .base import listxattr, getxattr, setxattr from .base import acl_get, acl_set from .base import set_flags, get_flags + from .base import on_different_mounts from .windows import SyncFile from .windows import set_times # type: ignore[no-redef] from .windows import process_alive, local_pid_alive diff --git a/src/borg/platform/base.py b/src/borg/platform/base.py index 206ce3a88..0cbcafd59 100644 --- a/src/borg/platform/base.py +++ b/src/borg/platform/base.py @@ -150,6 +150,18 @@ def get_flags(path, st, fd=None): return getattr(st, "st_flags", 0) +def on_different_mounts(fd1, fd2): + """ + Return whether the open file descriptors *fd1* and *fd2* are on different mounts. + + Returns True or False, or None if this can not be determined. + + This generic implementation compares st_dev. That is only correct on platforms where every + st_dev is a separately mounted filesystem (e.g. no btrfs-like subvolumes with their own st_dev). + """ + return os.fstat(fd1).st_dev != os.fstat(fd2).st_dev + + def set_times(path, *, atime_ns, mtime_ns, birthtime_ns=None, fd=None, follow_symlinks=True): """ Set the timestamps of *path* (or of the open file descriptor *fd*, if given). diff --git a/src/borg/platform/linux.pyx b/src/borg/platform/linux.pyx index 7c8ac39a1..b2ef61124 100644 --- a/src/borg/platform/linux.pyx +++ b/src/borg/platform/linux.pyx @@ -205,6 +205,33 @@ def get_flags(path, st, fd=None): return bsd_flags +def _get_mnt_id(fd): + """Return the id of the mount the open file descriptor fd is on (Linux >= 3.15), or None.""" + try: + with open(f"/proc/self/fdinfo/{fd}", "rb") as f: + for line in f: + if line.startswith(b"mnt_id:"): + return int(line.split()[1]) + except (OSError, ValueError, IndexError): + pass + return None + + +def on_different_mounts(fd1, fd2): + """ + Return whether the open file descriptors *fd1* and *fd2* are on different mounts. + + Returns True or False, or None if this can not be determined. + + Comparing st_dev is not good enough on Linux, e.g. btrfs subvolumes have their own st_dev, + so we compare the mount ids. While both fds are open, both mounts exist, so their ids are unique. + """ + mnt_id1, mnt_id2 = _get_mnt_id(fd1), _get_mnt_id(fd2) + if mnt_id1 is None or mnt_id2 is None: + return None + return mnt_id1 != mnt_id2 + + def acl_use_local_uid_gid(acl): """Replace the user/group field with the local uid/gid if possible """ diff --git a/src/borg/testsuite/archive_test.py b/src/borg/testsuite/archive_test.py index 23069054c..32bf5d8ad 100644 --- a/src/borg/testsuite/archive_test.py +++ b/src/borg/testsuite/archive_test.py @@ -15,7 +15,7 @@ from ..archive import Archive, CacheChunkBuffer, DownloadPipeline, RobustUnpacke from ..archive import ITEM_KEYS, Statistics from ..archive import zero_chunk_flags, zero_chunk_id, zero_chunk_ids from ..archive import BackupOSError, BackupRaceConditionError, backup_io, backup_io_iter, get_item_uid_gid -from ..archive import stat_update_check, tar_acl_to_borg +from ..archive import stat_update_check, is_automount, tar_acl_to_borg from ..helpers import msgpack, StableDict from ..repoobj import RepoObj from ..item import Item, ArchiveItem @@ -702,6 +702,27 @@ def test_stat_update_check_race_conditions(tmpdir): stat_update_check(st_file, st_other) +@pytest.mark.parametrize("mounted", [False, True, None]) +def test_is_automount(tmp_path, monkeypatch, mounted): + import borg.archive as archive_module + + monkeypatch.setattr(archive_module, "on_different_mounts", lambda fd1, fd2: mounted) + (tmp_path / "dir1").mkdir() + (tmp_path / "dir2").mkdir() + (tmp_path / "file").touch() + st_dir1, st_dir2, st_file = (os.stat(tmp_path / name) for name in ("dir1", "dir2", "file")) + parent_fd, fd = 3, 4 # only passed through to on_different_mounts + # a different directory is only accepted if it is on a different mount than its parent: + assert is_automount(st_dir1, st_dir2, parent_fd, fd) is (mounted is True) + # the same directory is no automount, but the normal case: + assert is_automount(st_dir1, st_dir1, parent_fd, fd) is False + # a file type change is never accepted: + assert is_automount(st_file, st_dir2, parent_fd, fd) is False + assert is_automount(st_dir1, st_file, parent_fd, fd) is False + # without a parent fd (recursion roots), we can not check: + assert is_automount(st_dir1, st_dir2, None, fd) is False + + def test_get_item_uid_gid(): # test requires that: # - a user/group name for the current process' real uid/gid exists. diff --git a/src/borg/testsuite/archiver/create_cmd_test.py b/src/borg/testsuite/archiver/create_cmd_test.py index 82b66e12a..3fdcc6552 100644 --- a/src/borg/testsuite/archiver/create_cmd_test.py +++ b/src/borg/testsuite/archiver/create_cmd_test.py @@ -1568,6 +1568,56 @@ def test_create_exclude_dataless(archivers, request, monkeypatch): assert "A input/cloudfile" in output +@pytest.mark.skipif(is_win32, reason="no fd-based directory checks on windows") +@pytest.mark.parametrize("mounted", [False, True]) +@pytest.mark.parametrize("one_file_system", [False, True]) +def test_create_automount(archivers, request, monkeypatch, mounted, one_file_system): + """Opening an automount point mounts a filesystem there, so stat and fstat differ, see #6652.""" + import borg.archive as archive_module + import borg.archiver.create_cmd as create_cmd_module + from ...helpers import BackupRaceConditionError + + archiver = request.getfixturevalue(archivers) + if archiver.EXE: + pytest.skip("Skipping binary test due to patch objects") + create_regular_file(archiver.input_path, "automnt/file", contents=b"hello") + cmd(archiver, "repo-create", RK_ENCRYPTION) + + class AutomountPointStat: + # stat() of an automount point: a different inode (here even on a different st_dev, like + # for an autofs direct mount) than the root directory of the filesystem mounted there. + def __init__(self, st): + self._st = st + self.st_ino = st.st_ino + 1000000 + self.st_dev = st.st_dev + 1 + + def __getattr__(self, name): + return getattr(self._st, name) + + real_os_stat = create_cmd_module.os_stat + + def fake_os_stat(*, path=None, parent_fd=None, name=None, follow_symlinks=False): + st = real_os_stat(path=path, parent_fd=parent_fd, name=name, follow_symlinks=follow_symlinks) + return AutomountPointStat(st) if name == "automnt" else st + + monkeypatch.setattr(create_cmd_module, "os_stat", fake_os_stat) + monkeypatch.setattr(archive_module, "on_different_mounts", lambda fd1, fd2: mounted) + args = ["create", "--list", "test", "input"] + (["--one-file-system"] if one_file_system else []) + if mounted: + # we archive the mounted filesystem and recurse into it (because it is on the same st_dev as + # input, even with --one-file-system - the automount point would have been on another st_dev). + output = cmd(archiver, *args) + assert "race condition" not in output + assert "input/automnt/file" in cmd(archiver, "list", "test", "--short") + else: + # no mount there, so this is a real race condition and we skip the directory. + exc = BackupRaceConditionError("file inode changed (race condition), skipping file") + output = cmd(archiver, *args, exit_code=BackupWarning("input/automnt", exc).exit_code) + assert "input/automnt: file type or inode changed" in output + assert "E input/automnt\n" in output + assert "input/automnt" not in cmd(archiver, "list", "test", "--short") + + def test_exclude_nodump_dir_with_file(archivers, request): """A directory flagged NODUMP and its contents must not be archived.""" archiver = request.getfixturevalue(archivers) diff --git a/src/borg/testsuite/platform/all_test.py b/src/borg/testsuite/platform/all_test.py index be789b591..761d5b914 100644 --- a/src/borg/testsuite/platform/all_test.py +++ b/src/borg/testsuite/platform/all_test.py @@ -1,6 +1,10 @@ import io +import os -from ...platform import swidth, SyncFile +import pytest + +from ...platform import swidth, SyncFile, on_different_mounts +from ...platformflags import is_win32 def test_swidth_ascii(): @@ -38,3 +42,24 @@ def test_syncfile_close_idempotent(tmp_path): sf.write(b"data") sf.close() sf.close() # must not raise + + +@pytest.mark.skipif(is_win32, reason="can not open directories on windows") +def test_on_different_mounts(tmp_path): + (tmp_path / "subdir").mkdir() + # a directory that is usually a separately mounted filesystem (devtmpfs, devfs, procfs, tmpfs, ...): + candidates = [p for p in ("/dev", "/proc", "/tmp") if os.path.isdir(p)] + other_mount = next((p for p in candidates if os.stat(p).st_dev != os.stat("/").st_dev), None) + fds = {} + try: + for path in "/", other_mount, tmp_path, tmp_path / "subdir": + if path is not None: + fds[path] = os.open(path, os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) + assert on_different_mounts(fds[tmp_path], fds[tmp_path / "subdir"]) is False + assert on_different_mounts(fds[tmp_path], fds[tmp_path]) is False + if other_mount is None: + pytest.skip("found no separately mounted filesystem") + assert on_different_mounts(fds["/"], fds[other_mount]) is True + finally: + for fd in fds.values(): + os.close(fd)