add fileflags option for BSD and others#894
Open
tridge wants to merge 1 commit into
Open
Conversation
tridge
force-pushed
the
pr-fileflags
branch
2 times, most recently
from
2b2734a to
679ad50
Compare
tridge
added a commit
to tridge/rsync
that referenced
this pull request
May 20, 2026
Three independent CI failures on PR RsyncProject#894 commit ac1d6a9: 1. Ubuntu / Ubuntu 22.04 (5 test failures each in make check30): testsuite/rsync.fns quoted "$RSYNC" when probing -VV to detect whether the build supports fileflags. $RSYNC is a command STRING (e.g. "/path/to/rsync --protocol=30"), not a single binary path -- so the quoted form tries to exec a file whose name contains a space, fails silently, and the all_plus/dots widths fall through to the 9-char non-fileflags defaults. On Linux+chattr the rsync build emits 10-char (12-column %i) output, so devices, exclude, itemize, etc. mismatched their expected output. Fix: drop the quotes so shell word-splitting separates the binary from --protocol. Local check30 now clean. 2. NetBSD / OpenBSD (fileflags FAIL): set_fileflags() opened with O_RDONLY|O_NOFOLLOW|O_NONBLOCK and then fchflags'd. Both NetBSD and OpenBSD reject open() with O_NONBLOCK on a directory, returning EISDIR even though POSIX allows opening directories O_RDONLY. Drop O_NONBLOCK: it isn't needed -- rsync_fchflags() rejects everything except S_IFREG and S_IFDIR, and callers already short-circuit on S_ISLNK, so the open target is always a plain file or directory. Neither blocks on plain O_RDONLY. 3. AlmaLinux 8 (RSYNC_EXPECT_SKIPPED mismatch): The base image doesn't have chattr(1), so the fileflags test self-skips with "No chflags(1) or chattr(1) command on this host" even though the rsync build has FS_IOC_GETFLAGS support. Add fileflags back to the AlmaLinux skip-allowlist; comment notes the reason. (Could alternatively dnf install e2fsprogs in the workflow, but the skip is also fine -- Ubuntu / FreeBSD / macOS cover the test on every other CI job.) The remaining 4 CI jobs (Cygwin, FreeBSD, Solaris, macOS) were green on the round-2 push and unaffected by any of these fixes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tridge
force-pushed
the
pr-fileflags
branch
2 times, most recently
from
f973e48 to
eed2737
Compare
Add a --fileflags option to preserve file flags across a transfer:
chflags(2) on the BSDs/macOS and chattr-style flags (FS_IOC_{GET,SET}FLAGS)
on Linux. Originally based on rsync-patches/fileflags.diff (BSD-only),
then reworked for portability and hardened against a hostile sender.
Rebased onto current master (post-3.4.3), whose CVE fixes (CVE-2026-29518
+ family) overlap heavily with the security work here: master added
do_chmod_at / do_lchown_at / secure_relative_open hardening that this
builds on. The fileflags work keeps the original do_chmod / do_lchown
signatures, so master's _at variants and call sites are unchanged.
Options:
--fileflags preserve file flags. A SAFE_FILEFLAGS mask
(UF_NODUMP|UF_IMMUTABLE|UF_APPEND[|UF_HIDDEN]) is
applied by default; sender-supplied SF_* and
UF_NOUNLINK are dropped to avoid a DoS where a
hostile source pins permanent flags on the receiver.
--unsafe-fileflags widen the mask to the full sender value.
--force-change clear USR_IMMUTABLE (UF_*) on dest files being
updated/deleted so the op can proceed.
--force-uchange alias for --force-change.
--force-schange also clear SYS_IMMUTABLE (SF_*); separate opt-in.
--no-force-{u,s,}change clear those bits.
Implementation:
- lib/fileflags.c: portable rsync_fchflags / rsync_fgetflags /
rsync_lgetflags / stat_x_get_fileflags + BSD<->Linux bit
translation. Wire format is BSD bits. The Linux side does a
read-modify-write of just LINUX_WIRE_MASK so the kernel doesn't
reject the call when fs-internal bits (e.g. FS_EXTENT_FL) would
otherwise be cleared.
- stat_x grows a (fileflags, fileflags_cached) pair so the per-file
open()+ioctl cost on Linux happens at most once per stat_x life;
init_stat_x() zeroes both.
- syscall.c do_unlink / do_rmdir / do_chmod / do_lchown / do_rename
grow dirfd-anchored force_change recovery via
force_change_open_parent / force_change_open_target + fchflags /
fchmod / fchown / unlinkat / renameat. Recovery opens through
secure_relative_open(curr_dir, dirpart, O_RDONLY|O_DIRECTORY|
O_NOFOLLOW) so RESOLVE_BENEATH (where available) bounds the
operation to the destination subtree; symlinks are rejected at open.
- generator.c defers clearing a directory's immutable flag until its
contents are processed, then re-applies it afterwards.
- Daemon mode refuses fileflags / unsafe-fileflags / force-change /
force-uchange / force-schange / no-force-uchange / no-force-schange
by default; opt-in per-module via "refuse options = !fileflags".
set_refuse_options also gains a POPT_BIT_SET/POPT_BIT_CLR fix: the
old check compared op->argInfo == POPT_ARG_VAL literally and missed
POPT_BIT_SET (POPT_ARG_VAL|POPT_ARGFLAG_OR), letting refused bit-set
options slip through; it now masks via POPT_ARG_MASK.
- batch mode records the fileflags setting in the batch flags.
- The Linux ioctl path is gated behind an autoconf check for
FS_IOC_GETFLAGS / FS_IOC_SETFLAGS / FS_NODUMP_FL / FS_IMMUTABLE_FL /
FS_APPEND_FL in <linux/fs.h>; it falls through to "no fileflags
support" when absent.
Tests (ported to the Python testsuite that replaced the shell suite):
- testsuite/fileflags_test.py picks chflags(1) or chattr(1) by
availability; on Linux it parses lsattr down to the transferable
letters (a, d, i, u). The uchg / deferred-immutable / force-change
blocks self-skip on Linux non-root (CAP_LINUX_IMMUTABLE required).
- testsuite/daemon-refuse-fileflags_test.py covers the daemon refusal
and the per-module opt-in, driving the daemon via
rsyncfns.start_test_daemon (secure stdio-pipe by default). It captures
the client's stderr to a file rather than a subprocess PIPE: the
RSYNC_CONNECT_PROG transport forks a daemon that inherits the stderr fd,
and draining that as a pipe hung the test for 300s on OpenBSD when the
forked daemon lingered holding the write end (the refusals themselves
worked). A per-client timeout turns any real hang into a clean failure.
- testsuite/rsyncfns.py: a fileflags build emits a 12-char %i itemize
string (an extra trailing 'f' column), so all_plus / allspace / dots
widen by one; detect it from rsync -VV so the itemize / exclude /
devices tests stay correct on both fileflags and non-fileflags builds.
- CI workflows: fileflags self-skips where chattr is absent (AlmaLinux 8)
or the build lacks fileflags support (Cygwin); both fileflags tests
self-skip under protocol 29 (varint flag encoding needs protocol >= 30).
Verified on Linux/ext4: full suite 58 passed / 4 skipped / 0 failed; the
fileflags test's immutable, deferred-immutable-directory and force-change
blocks pass under root.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is based on the fileflags.patch in the rsync-patches archive
needs a bit more work before merging
todo: