Skip to content

do not archive ACLs that are equivalent to the mode bits (and fix FreeBSD default ACL loss) - #10040

Draft
ThomasWaldmann wants to merge 2 commits into
borgbackup:masterfrom
ThomasWaldmann:acl-default-only
Draft

do not archive ACLs that are equivalent to the mode bits (and fix FreeBSD default ACL loss)#10040
ThomasWaldmann wants to merge 2 commits into
borgbackup:masterfrom
ThomasWaldmann:acl-default-only

Conversation

@ThomasWaldmann

Copy link
Copy Markdown
Member

Follow-up to #9954: a directory that has only a default ACL got a system.posix_acl_access xattr archived and exposed by borg mount, although neither the source filesystem nor the kernel has such an xattr for it. Looking into it turned up the same class of problem on FreeBSD, where it is worse. One commit per platform.

1. Linux: do not archive ACLs that are equivalent to the mode bits

acl_extended_file_nofollow() / acl_extended_fd() also consider the default ACL of a directory, so they return 1 for a directory that only has a default ACL. acl_get() took that as "there is an extended ACL" and archived the access ACL too - which for such a directory is just the traditional permission bits:

acl_access:  b'user::rwx\ngroup::---\nother::---'

The kernel keeps no system.posix_acl_access xattr for a mode-equivalent ACL, so the mount was offering an xattr that does not exist on the source. The mirror image of the same wart: a directory without a default ACL got acl_default = b'' archived (acl_to_any_text() of an empty ACL).

Now acl_access is only archived if acl_equiv_mode(acl, NULL) != 0 (-1 = error still archives, so an odd ACL is never silently dropped) and acl_default only if acl_entries(acl) > 0.

Extraction is unaffected: a skipped mode-equivalent access ACL is exactly the mode bits, and a skipped empty default ACL is what acl_set_file(ACL_TYPE_DEFAULT, <empty>) produced anyway.

Additionally, the FUSE mounts now skip trivial/empty ACLs (new generic acl_is_extended() in platform/base.py, used by fuse.py and hlfuse.py), so archives created by older borg versions stop exposing these xattrs too.

2. FreeBSD: do not lose the default ACL of a directory

The same starting point, but FreeBSD's libc behaves the other way round - and the result is data loss rather than noise.

acl_extended_link_np() inspects only the access (resp. NFSv4) ACL; see _acl_extended_file() in lib/libc/posix1e/acl_extended_file_np.c, which runs acl_is_trivial_np() on ACL_TYPE_ACCESS. So a directory that only has a default ACL does not count as "extended", acl_get() returned early, and the default ACL was never archived at all - restoring such a directory silently loses its inheritance policy.

That call now only gates the access / NFSv4 ACL. For a directory on a POSIX.1e filesystem the default ACL is looked at in any case, using _PC_ACL_EXTENDED - the very probe acl_set() already uses on the restore side, so both directions now ask the same question.

Also, empty ACLs are no longer archived: for a directory without a default ACL the kernel returns success and an empty ACL (see ufs_getacl_posix1e() in sys/ufs/ufs/ufs_acl.c), which borg stored as acl_default = b''.

Cost: on an acls-mounted UFS every directory now gets one default-ACL fetch, and on ZFS every directory gets one extra lpathconf - unavoidable if we must not rely on a call that cannot see default ACLs.

Behaviour per case (FreeBSD): file without ACLs still returns early; file/dir with an access ACL unchanged; ZFS/NFSv4 still takes the NFS4 branch (only reachable when extended, so no change); dir with only a default ACL now archives it; dir with no ACLs still yields {}. On a filesystem without ACL support acl_extended_link_np() still fails with EOPNOTSUPP before any of this, which archive.py swallows as before.

Other platforms

  • macOS: fine. acl_get() only deals with ACL_TYPE_EXTENDED (no access/default split) and macOS returns NULL/ENOENT when there is no ACL, which darwin.pyx already handles. Checked on a Mac: plain dir -> {}, dir with an ACL -> only the real entry.
  • NetBSD / OpenBSD / Windows / generic POSIX: wired to the base.py no-op acl_get/acl_set, nothing to do.
  • The mount-side exposure is Linux-only either way (ACL_XATTRS = {...} if is_linux else {}).

Tests

  • platform/linux_test.py: test_default_acl_only, test_access_acl_only_no_empty_default, test_acl_is_extended.
  • platform/freebsd_test.py: the two analogous ACL tests.
  • archiver/mount_cmds_test.py::test_fuse_acls: extended with a dir2 that has only a default ACL - asserts the access xattr is neither listed nor readable through the mount, that the default xattr matches the source byte for byte, and that acl_get() through the mount sees no access ACL.

Both new Linux platform tests fail on master with exactly the two bogus values above, and pass with the fix.

Verification status

Linux is fully verified: the platform tests and the full test_fuse_acls (including the ACL-xattr passthrough assertions, which need a container outside a user namespace - inside one the kernel refuses getxattr of system.posix_acl_* on FUSE mounts, as noted in #9954) pass, along with platform/, mount_cmds, fuse, create, extract, diff and item.

The FreeBSD commit is untested on real FreeBSD - I had no FreeBSD system at hand. It is derived from the freebsd-src sources quoted above and the .pyx was Cython-translated cleanly, but that is a syntax check, not a behaviour check. Note also that the FreeBSD vm_tests job is continue-on-error and the ACL tests are skipif_acls_not_working, so they will skip unless that VM's filesystem is mounted with acls. Please give it a run on a FreeBSD box with mount -o acls before trusting that commit.

A directory that only has a default ACL still counts as "extended" for
acl_extended_file_nofollow/acl_extended_fd, so borg archived its access
ACL, although that ACL just mirrors the traditional permission bits -
the kernel does not keep a system.posix_acl_access xattr for it either.
"borg mount" then offered such a bogus xattr. Same the other way round:
a directory without a default ACL got an empty acl_default archived.

Now only archive the access ACL if it is not equivalent to the mode bits
(acl_equiv_mode) and the default ACL if it has any entries (acl_entries).

The FUSE mounts additionally skip trivial/empty ACLs (acl_is_extended),
so archives created by older borg versions do not expose these xattrs
either.
acl_extended_link_np() only inspects the access (resp. NFSv4) ACL - see
_acl_extended_file() in lib/libc/posix1e/acl_extended_file_np.c, which
runs acl_is_trivial_np() on ACL_TYPE_ACCESS. So a directory that has
only a default ACL does not count as "extended", borg returned early and
never archived that default ACL at all - restoring it silently lost the
inheritance policy.

Now acl_extended_link_np() only gates the access / NFSv4 ACL, and for a
directory on a POSIX.1e filesystem (checked via _PC_ACL_EXTENDED, just
like acl_set does already) the default ACL is looked at in any case.

Also, do not archive empty ACLs: for a directory without a default ACL
the kernel returns success and an empty ACL (see ufs_getacl_posix1e()),
so borg stored acl_default = b'' for those.
@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.47619% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.19%. Comparing base (3a8431c) to head (97e41d0).
⚠️ Report is 3 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/borg/platform/base.py 85.71% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10040      +/-   ##
==========================================
+ Coverage   86.18%   86.19%   +0.01%     
==========================================
  Files          96       96              
  Lines       17434    17449      +15     
  Branches     2665     2671       +6     
==========================================
+ Hits        15025    15040      +15     
  Misses       1668     1668              
  Partials      741      741              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@ThomasWaldmann
ThomasWaldmann marked this pull request as draft August 5, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant