From 14ab548efc6d46af8f2f197279130290bd246481 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sun, 14 Jun 2026 08:02:20 +1000 Subject: [PATCH 01/16] runtests: write valgrind logs to a world-writable subdir Under --valgrind some tests run rsync with reduced privileges: partial_nowrite wraps it in "setpriv --inh-caps -all --bounding-set -all" to force EACCES, and chdir-symlink-race's daemon drops to the module's uid. Such a child cannot create valgrind's --log-file in a root-owned scratchbase, so valgrind aborts at startup and the test fails (seen only in the root + --use-tcp cell). Put the logs in a 1777 valgrind-logs/ subdir so a privilege-dropped child can always write them. Scan and cleanup are unchanged; the logs just move one directory down. --- runtests.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/runtests.py b/runtests.py index 2c7a7f9b..809276e3 100755 --- a/runtests.py +++ b/runtests.py @@ -266,7 +266,14 @@ def build_rsync_cmd(rsync_bin, args, scratchbase): """Build the RSYNC command string for tests.""" parts = [] if args.valgrind: - vlog = os.path.join(scratchbase, 'valgrind.%p.log') + # Logs go in a world-writable+sticky subdir so that rsync children + # which drop privileges (the setpriv cap-drop in partial_nowrite, a + # daemon dropping to the module's uid) can still create their log file + # even when scratchbase itself is root-owned. + vgdir = os.path.join(scratchbase, 'valgrind-logs') + os.makedirs(vgdir, exist_ok=True) + os.chmod(vgdir, 0o1777) + vlog = os.path.join(vgdir, 'valgrind.%p.log') vopts = f'--log-file={vlog}' supp = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'testsuite', 'valgrind.supp') @@ -454,7 +461,7 @@ def main(): print(f' os={subprocess.check_output(["uname", "-a"], text=True).strip()}') print(f' preserve_scratch={"yes" if args.preserve_scratch else "no"}') if args.valgrind: - print(f' valgrind=enabled (logs in valgrind.*.log)') + print(f' valgrind=enabled (logs in valgrind-logs/valgrind.*.log)') if args.parallel > 1: print(f' parallel={args.parallel}') print(f' daemon_transport={"tcp (loopback)" if args.use_tcp else "pipe (secure default)"}') @@ -620,7 +627,7 @@ def main(): # Check valgrind logs for errors vg_errors = 0 if args.valgrind: - for vlog in sorted(glob.glob(os.path.join(scratchbase, 'valgrind.*.log'))): + for vlog in sorted(glob.glob(os.path.join(scratchbase, 'valgrind-logs', 'valgrind.*.log'))): try: with open(vlog) as f: content = f.read() @@ -644,7 +651,7 @@ def main(): if skipped > 0: print(f' {skipped} skipped') if vg_errors > 0: - print(f' {vg_errors} valgrind error(s) found (see logs in {scratchbase})') + print(f' {vg_errors} valgrind error(s) found (see logs in {os.path.join(scratchbase, "valgrind-logs")})') if expect is not None: # Version-mixing mode: the run is judged purely on whether each test's -- 2.47.3 From a27095f0e19e6a5232579f204825eb7bcba899a4 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 15 Jun 2026 13:53:04 +1000 Subject: [PATCH 02/16] Create FUNDING.yml --- .github/FUNDING.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .github/FUNDING.yml diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml new file mode 100644 index 00000000..3e2f5489 --- /dev/null +++ b/.github/FUNDING.yml @@ -0,0 +1,4 @@ +# These are supported funding model platforms + +github: tridge +patreon: AndrewTridgell -- 2.47.3 From fe93ffcd6537fdb88e7bbe5587ed0385e1680a96 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 15 Jun 2026 13:58:46 +1000 Subject: [PATCH 03/16] fix funding github username --- .github/FUNDING.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml index 3e2f5489..bd49de48 100644 --- a/.github/FUNDING.yml +++ b/.github/FUNDING.yml @@ -1,4 +1,4 @@ # These are supported funding model platforms -github: tridge +github: RsyncProject patreon: AndrewTridgell -- 2.47.3 From 399cf1aa5de9b2c757e481c3a34759a26a970f83 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 15 Jun 2026 14:50:42 +0200 Subject: [PATCH 04/16] generator: fix build warning in sum_sizes_sqroot() cnt is assigned a variable but never actually used, so remove it entirely as it's not needed anymore. --- generator.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/generator.c b/generator.c index 107c925a..8642236e 100644 --- a/generator.c +++ b/generator.c @@ -718,8 +718,7 @@ static void sum_sizes_sqroot(struct sum_struct *sum, int64 len) else { int32 max_blength = protocol_version < 30 ? OLD_MAX_BLOCK_SIZE : MAX_BLOCK_SIZE; int32 c; - int cnt; - for (c = 1, l = len, cnt = 0; l >>= 2; c <<= 1, cnt++) {} + for (c = 1, l = len; l >>= 2; c <<= 1) {} if (c < 0 || c >= max_blength) blength = max_blength; else { -- 2.47.3 From 412cddf6bec5a7464359c60260640ee473c1e8e0 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 12 Jun 2026 10:53:01 +1000 Subject: [PATCH 05/16] scan-build: zero-init buffers the analyzer can't prove are written clang's static analyzer doesn't model SIVAL/SIVAL64/SIVALu or getpeername/getsockname as initializing their target bytes, so it reports false "garbage value" reads. Zero-init the affected buffers; the bytes are always overwritten at runtime, so this only quiets the analyzer. io.c: write_varint/write_varlong b[] hashtable.c: hash_search buf[] socket.c: accepted_peer/our_local --- hashtable.c | 2 +- io.c | 4 ++-- socket.c | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/hashtable.c b/hashtable.c index f4aa85f1..4f8929d6 100644 --- a/hashtable.c +++ b/hashtable.c @@ -120,7 +120,7 @@ void *hashtable_find(struct hashtable *tbl, int64 key, void *data_when_new) if (!key64) { /* Based on Jenkins One-at-a-time hash. */ - uchar buf[4], *keyp = buf; + uchar buf[4] = {0}, *keyp = buf; /* {0} only to satisfy the analyzer (SIVALu fills buf) */ int i; SIVALu(buf, 0, key); diff --git a/io.c b/io.c index 0b96c270..2dfeeaa3 100644 --- a/io.c +++ b/io.c @@ -2163,7 +2163,7 @@ void write_int(int f, int32 x) void write_varint(int f, int32 x) { - char b[5]; + char b[5] = {0}; /* {0} only to satisfy the analyzer: it doesn't model SIVAL initialising b[1..4] */ uchar bit; int cnt; @@ -2185,7 +2185,7 @@ void write_varint(int f, int32 x) void write_varlong(int f, int64 x, uchar min_bytes) { - char b[9]; + char b[9] = {0}; /* {0} only to satisfy the analyzer: it doesn't model SIVAL64 initialising b[1..8] */ uchar bit; int cnt = 8; diff --git a/socket.c b/socket.c index d5aa0cb7..4ac79ec7 100644 --- a/socket.c +++ b/socket.c @@ -802,7 +802,8 @@ static int socketpair_tcp(int fd[2]) * the local address of our connecting end (fd[1]), and both must be * loopback. If they differ, someone else connected first; fail closed. */ { - struct sockaddr_in accepted_peer, our_local; + /* {0}: the analyzer doesn't model getpeername/getsockname filling these. */ + struct sockaddr_in accepted_peer = {0}, our_local = {0}; socklen_t plen = sizeof accepted_peer; socklen_t llen = sizeof our_local; -- 2.47.3 From 8118744f2a87bf286d5d1961b3dfff9484261989 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 16 Jun 2026 06:43:20 +1000 Subject: [PATCH 06/16] scan-build: drop dead assignments Remove stores that are never read before being overwritten or going out of scope. No behavior change except batch.c write_opt, which now accumulates the leading-space write error into the return value (consistent with the arg branch) instead of discarding it. simd-checksum-x86_64.cpp, options.c, util1.c, batch.c --- batch.c | 2 +- options.c | 1 - simd-checksum-x86_64.cpp | 4 ++-- util1.c | 2 +- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/batch.c b/batch.c index 878b6dd1..c2ef85ea 100644 --- a/batch.c +++ b/batch.c @@ -194,7 +194,7 @@ static int write_opt(const char *opt, const char *arg) { int len = strlen(opt); int err = write(batch_sh_fd, " ", 1) != 1; - err = write(batch_sh_fd, opt, len) != len ? 1 : 0; + err |= write(batch_sh_fd, opt, len) != len; if (arg) { err |= write(batch_sh_fd, "=", 1) != 1; err |= write_arg(arg); diff --git a/options.c b/options.c index 3c2d2352..8568af2b 100644 --- a/options.c +++ b/options.c @@ -1489,7 +1489,6 @@ int parse_arguments(int *argc_p, const char ***argv_p) *argc_p = 0; } else if (poptDupArgv(argc, argv, argc_p, argv_p) != 0) out_of_memory("parse_arguments"); - argv = *argv_p; poptFreeContext(pc); am_starting_up = 0; diff --git a/simd-checksum-x86_64.cpp b/simd-checksum-x86_64.cpp index 7b0a7546..da48a7d3 100644 --- a/simd-checksum-x86_64.cpp +++ b/simd-checksum-x86_64.cpp @@ -488,8 +488,8 @@ static inline uint32 get_checksum1_cpp(char *buf1, int32 len) // multiples of 32 bytes using SSE2 (if available) i = get_checksum1_sse2_32((schar*)buf1, len, i, &s1, &s2); - // whatever is left - i = get_checksum1_default_1((schar*)buf1, len, i, &s1, &s2); + // whatever is left (updates s1/s2; the returned offset is unused here) + get_checksum1_default_1((schar*)buf1, len, i, &s1, &s2); return (s1 & 0xffff) + (s2 << 16); } diff --git a/util1.c b/util1.c index 7a1f24b5..d9d9f4bc 100644 --- a/util1.c +++ b/util1.c @@ -524,7 +524,7 @@ int robust_unlink(const char *fname) snprintf(&path[pos], MAX_RENAMES_DIGITS+1, "%03d", counter); if (++counter >= MAX_RENAMES) counter = 1; - } while ((rc = access(path, 0)) == 0 && counter != start); + } while (access(path, 0) == 0 && counter != start); if (INFO_GTE(MISC, 1)) { rprintf(FWARNING, "renaming %s to %s because of text busy\n", -- 2.47.3 From 4e67f8747952c96b9008a015e8feafc5ad18b8d4 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Fri, 12 Jun 2026 10:53:10 +1000 Subject: [PATCH 07/16] scan-build: fix resource leaks on error paths clientserver.c: close the --early-input-file FILE* on the fstat/oversize/early-EOF error returns; it was only closed on the success path. getgroups.c: free the gid list before returning. --- clientserver.c | 4 ++++ getgroups.c | 1 + 2 files changed, 5 insertions(+) diff --git a/clientserver.c b/clientserver.c index cc59663a..dbde116c 100644 --- a/clientserver.c +++ b/clientserver.c @@ -270,11 +270,14 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char FILE *f = fopen(early_input_file, "rb"); if (!f || do_fstat(fileno(f), &st) < 0) { rsyserr(FERROR, errno, "failed to open %s", early_input_file); + if (f) + fclose(f); return -1; } early_input_len = st.st_size; if (early_input_len > (int)sizeof line) { rprintf(FERROR, "%s is > %d bytes.\n", early_input_file, (int)sizeof line); + fclose(f); return -1; } if (early_input_len > 0) { @@ -283,6 +286,7 @@ int start_inband_exchange(int f_in, int f_out, const char *user, int argc, char int len; if (feof(f)) { rprintf(FERROR, "Early EOF in %s\n", early_input_file); + fclose(f); return -1; } len = fread(line, 1, early_input_len, f); diff --git a/getgroups.c b/getgroups.c index 8a37ed0b..6b83d452 100644 --- a/getgroups.c +++ b/getgroups.c @@ -57,5 +57,6 @@ printf("%lu", (unsigned long)gid); printf("\n"); + free(list); return 0; } -- 2.47.3 From 3f5884a3bb88e6ee568708184dda16e483b53775 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 16 Jun 2026 06:43:20 +1000 Subject: [PATCH 08/16] scan-build: close a test-helper FILE* leak wildtest.c: close the test file before main() returns (a real, if exit-benign, FILE* leak flagged by scan-build). --- wildtest.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wildtest.c b/wildtest.c index 10cab003..df87f924 100644 --- a/wildtest.c +++ b/wildtest.c @@ -210,6 +210,8 @@ main(int argc, char **argv) string[0], string[1]); } + fclose(fp); + if (!wildmatch_errors) fputs("No", stdout); else -- 2.47.3 From 5553271274f4426ac18fc98087f91670ec4d4305 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Tue, 16 Jun 2026 07:54:07 +1000 Subject: [PATCH 09/16] ci: run scan-build on pinned clang-18 + latest clang (informational) Split the scan-build workflow into two non-gating jobs, each uploading its HTML report as an artifact: - pinned-clang18: clang-18 / clang-tools-18 on ubuntu-24.04, so the checker set -- and thus the report -- is deterministic. - informational-latest: whatever clang ubuntu-latest ships, to surface what newer analyzers see. Both are informational (no --status-bugs): the tree still has known clang-18 findings, so the run reports without blocking the build. Once the tree is at zero for clang-18, re-add --status-bugs to the pinned job to turn it back into a gate. Installs libpopt-dev so configure finds popt under the scan-build compiler wrapper. --- .github/workflows/scan-build.yml | 63 +++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 14 deletions(-) diff --git a/.github/workflows/scan-build.yml b/.github/workflows/scan-build.yml index 23c1b73c..42cb07b8 100644 --- a/.github/workflows/scan-build.yml +++ b/.github/workflows/scan-build.yml @@ -14,9 +14,15 @@ on: workflow_dispatch: jobs: - scan-build: - runs-on: ubuntu-latest - name: rsync scan-build (clang analyzer) + # PINNED run: clang-18 on a pinned runner (ubuntu-24.04, whose apt repos carry + # clang-18/clang-tools-18) so the checker set -- and thus the report -- is + # deterministic. Informational for now: the tree still has known clang-18 + # findings, so this surfaces the report without blocking. Once the tree is at + # zero for clang-18, re-add --status-bugs to the scan-build step below to turn + # this back into a gate. + pinned-clang18: + runs-on: ubuntu-24.04 + name: scan-build (clang-18, pinned) steps: - uses: actions/checkout@v4 with: @@ -24,28 +30,57 @@ jobs: - name: prep run: | sudo apt-get update - sudo apt-get install -y clang clang-tools acl libacl1-dev attr libattr1-dev liblz4-dev libzstd-dev libxxhash-dev openssl + sudo apt-get install -y clang-18 clang-tools-18 acl libacl1-dev attr libattr1-dev liblz4-dev libzstd-dev libxxhash-dev libpopt-dev openssl - name: configure (under scan-build) # Run configure under scan-build so its analyzer compiler-wrapper is baked # into the Makefile's $(CC); --disable-md2man avoids the doc toolchain. + run: scan-build-18 ./configure --with-rrsync --disable-md2man + - name: scan-build (pinned clang-18) + # Informational: no --status-bugs, so existing findings don't fail the + # build; the report is summarised and uploaded for triage. Re-add + # --status-bugs here (and 'set -o pipefail; ...; exit $status') to gate + # once the tree is at zero for clang-18. + run: | + scan-build-18 -o "$PWD/scan-report" make check-progs -j"$(nproc)" 2>&1 | tee scan-build.out + echo '## scan-build (clang-18, pinned)' >>"$GITHUB_STEP_SUMMARY" + grep -E 'scan-build: .* bugs? found|scan-build: No bugs found' scan-build.out >>"$GITHUB_STEP_SUMMARY" || true + - name: upload report + if: always() + uses: actions/upload-artifact@v4 + with: + name: scan-build-report-clang18 + path: scan-report + if-no-files-found: ignore + + # INFORMATIONAL run: whatever clang ubuntu-latest currently ships. Newer + # clang releases enable extra, FP-heavy checkers that the gate deliberately + # avoids, so this is NOT a gate (no --status-bugs). It surfaces what the + # newest analyzer sees -- useful for spotting genuine new findings before a + # gate bump -- without blocking merges. continue-on-error keeps a noisy or + # broken run from affecting the workflow's required status. + informational-latest: + runs-on: ubuntu-latest + name: scan-build (latest clang, informational) + continue-on-error: true + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: prep + run: | + sudo apt-get update + sudo apt-get install -y clang clang-tools acl libacl1-dev attr libattr1-dev liblz4-dev libzstd-dev libxxhash-dev libpopt-dev openssl + - name: configure (under scan-build) run: scan-build ./configure --with-rrsync --disable-md2man - name: scan-build (informational) - # Static analysis only -- INFORMATIONAL, not a gate. rsync currently has - # a fair number of reports that are overwhelmingly known false positives - # (e.g. unix.Chroot "no chdir after chroot", core.NonNullParamChecker - # against functions that can't actually receive NULL). We publish the - # HTML report as an artifact and print the bug count to the run summary, - # but do NOT pass --status-bugs, so this surfaces new analyzer findings - # without going red on arrival. check-progs builds rsync + the test - # helpers without needing the man-page toolchain. run: | scan-build -o "$PWD/scan-report" make check-progs -j"$(nproc)" 2>&1 | tee scan-build.out - echo '## scan-build summary' >>"$GITHUB_STEP_SUMMARY" + echo '## scan-build informational (latest clang)' >>"$GITHUB_STEP_SUMMARY" grep -E 'scan-build: .* bugs? found|scan-build: No bugs found' scan-build.out >>"$GITHUB_STEP_SUMMARY" || true - name: upload report if: always() uses: actions/upload-artifact@v4 with: - name: scan-build-report + name: scan-build-report-latest path: scan-report if-no-files-found: ignore -- 2.47.3 From f5fa55672db9c65dffb8698eeab3173a93692e16 Mon Sep 17 00:00:00 2001 From: AlphaGlider25 <12aa44edsta@gmail.com> Date: Fri, 19 Jun 2026 03:30:48 +0200 Subject: [PATCH 10/16] Fix Solaris xattr retry handling Use the remaining byte count for retry writes and avoid using a size_t sentinel for write failures. --- lib/sysxattrs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/sysxattrs.c b/lib/sysxattrs.c index 5a6aeaad..861af981 100644 --- a/lib/sysxattrs.c +++ b/lib/sysxattrs.c @@ -227,19 +227,19 @@ int sys_lsetxattr(const char *path, const char *name, const void *value, size_t return -1; for (bufpos = 0; bufpos < size; ) { - ssize_t cnt = write(attrfd, (char*)value + bufpos, size); + ssize_t cnt = write(attrfd, (char*)value + bufpos, size - bufpos); if (cnt <= 0) { if (cnt < 0 && errno == EINTR) continue; - bufpos = -1; - break; + close(attrfd); + return -1; } bufpos += cnt; } close(attrfd); - return bufpos > 0 ? 0 : -1; + return 0; } int sys_lremovexattr(const char *path, const char *name) -- 2.47.3 From e2a24e85814accd3f54f594d04d96041ce7de507 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sat, 18 Jul 2026 17:13:11 +1000 Subject: [PATCH 11/16] testsuite: C23 bool compatibility --- wildtest.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/wildtest.c b/wildtest.c index df87f924..5867bb1c 100644 --- a/wildtest.c +++ b/wildtest.c @@ -23,6 +23,7 @@ #include "lib/wildmatch.c" #include +#include #ifdef COMPARE_WITH_FNMATCH #include @@ -32,10 +33,6 @@ int fnmatch_errors = 0; int wildmatch_errors = 0; -#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 202311L -typedef char bool; -#endif - int output_iterations = 0; int explode_mod = 0; int empties_mod = 0; -- 2.47.3 From e6956e0a30cb360b413ce2ad8df89204a1951059 Mon Sep 17 00:00:00 2001 From: Stuart Inglis Date: Wed, 1 Jul 2026 07:29:16 +1200 Subject: [PATCH 12/16] match: bound the hash_search() chain walk (issue #217) hash_search() walks the entire hash-table chain for the current rolling checksum at every byte offset of the source file. Disk and VM images contain large runs of identical blocks, so a single weak checksum (get_checksum1) can collide thousands of times and pile every one of those blocks onto one chain. When the sender then rolls across a region whose weak checksum keeps landing on that chain without ever producing a strong-checksum match, it re-walks the whole chain for every byte, giving O(file_size * chain_length) behaviour. The result is rsync sitting at 100% CPU for hours with no apparent progress -- the long-standing "rsync hangs on large files" reports. Cap the number of same-weak-checksum candidates examined per offset at MAX_CHAIN_LEN. Once the cap is hit we treat the offset as a non-match and roll forward a byte; any block skipped this way is simply sent as literal data, so the transferred result is always correct -- only the transfer size is marginally affected. This is purely a sender-side search limit: it changes no checksum, emitted byte, or protocol field, so a capped sender interoperates with an unmodified receiver and vice versa. On a synthetic 40000-block basis sharing one weak checksum, syncing a 60KB source dropped from ~18.4s to ~0.7s; the unbounded cost grows with the square of the file size. testsuite/hashsearch-chain_test.py reproduces the pathology with a tiny basis of weak-checksum-colliding decoy blocks and asserts, via the existing false_alarms counter (--debug=deltasum1), that the per-hash-hit chain walk stays bounded. The assertion is exact and machine-independent rather than timing-based. --- match.c | 32 +++++++++ testsuite/hashsearch-chain_test.py | 108 +++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 testsuite/hashsearch-chain_test.py diff --git a/match.c b/match.c index dfd6af2c..82a80a08 100644 --- a/match.c +++ b/match.c @@ -44,6 +44,29 @@ extern struct stats stats; #define TRADITIONAL_TABLESIZE (1<<16) +/* The maximum number of same-weak-checksum candidates we will compare + * against at a single file offset before giving up and rolling forward a + * byte. A weak checksum that collides thousands of times (very common in + * disk/VM images, which contain large runs of identical blocks) would + * otherwise turn hash_search()'s inner loop into an O(file_size * + * chain_length) scan, pegging a CPU at 100% for hours with no apparent + * progress (issue #217). + * + * Concretely, a synthetic 40000-block basis whose blocks all share one weak + * checksum took ~18.4s to sync a 60KB source on a modern x86_64 box before + * this cap and ~0.7s after it -- and the unbounded cost grows with the + * square of the file size, which is what produced the multi-hour "hangs" + * reported against real multi-GB images. + * + * Capping the per-offset work keeps the search bounded; any block we skip + * over is simply sent as literal data, so the result is always correct -- + * only the transfer size is (slightly) affected. This is purely a + * sender-side search limit: it changes no checksum, emitted byte, or + * protocol field, so a capped sender interoperates with any receiver. */ +#ifndef MAX_CHAIN_LEN +#define MAX_CHAIN_LEN 1024 +#endif + static uint32 tablesize; static int32 *hash_table; @@ -182,6 +205,7 @@ static void hash_search(int f,struct sum_struct *s, int done_csum2 = 0; uint32 hash_entry; int32 i, *prev; + int32 chain_len = 0; if (DEBUG_GTE(DELTASUM, 4)) { rprintf(FINFO, "offset=%s sum=%04x%04x\n", @@ -218,6 +242,14 @@ static void hash_search(int f,struct sum_struct *s, if (sum != s->sums[i].sum1) continue; + /* Bound the work spent on a single pathological hash + * bucket. If this weak checksum matches more than + * MAX_CHAIN_LEN records, stop scanning and treat this + * offset as a non-match (issue #217). The skipped data + * is sent literally, never corrupted. */ + if (++chain_len > MAX_CHAIN_LEN) + break; + /* also make sure the two blocks are the same length */ l = (int32)MIN((OFF_T)s->blength, len-offset); if (l != s->sums[i].len) diff --git a/testsuite/hashsearch-chain_test.py b/testsuite/hashsearch-chain_test.py new file mode 100644 index 00000000..9dcb45b4 --- /dev/null +++ b/testsuite/hashsearch-chain_test.py @@ -0,0 +1,108 @@ +#!/usr/bin/env python3 +"""Regression test for issue #217: hash_search() must not blow up to O(N^2). + +A disk/VM image can contain a huge number of blocks that share the same weak +rolling checksum (e.g. long runs of identical bytes). All such blocks land on +a single hash-table chain. When the sender then rolls across a region whose +weak checksum keeps landing on that chain but never produces a strong-checksum +match, the inner loop of hash_search() used to walk the *entire* chain for +every single byte offset. The transfer would peg one CPU at 100% for hours +with no visible progress -- "rsync hangs at 100% CPU". + +We reproduce a small, deterministic version of that pathology: + + * Destination/basis file: many identical "decoy" blocks. Each decoy is an + all-C block whose first three bytes are perturbed by (+1,-2,+1). That + perturbation leaves rsync's weak checksum (get_checksum1) unchanged but + changes the strong checksum, so every decoy shares one weak checksum yet + never strong-matches the source -> one very long chain, all false alarms. + + * Source file: a short run of the constant byte C. Every window has that + same weak checksum, so without a bound the sender walks the whole chain at + every offset. + +Rather than measure wall-clock time (which is hopelessly machine dependent), +we count the work directly. rsync's existing `false_alarms` counter -- shown +by `--debug=deltasum1` -- is incremented once per strong-checksum comparison +that fails, i.e. exactly once per chain entry examined and rejected. With the +per-offset cap in match.c the sender examines at most MAX_CHAIN_LEN entries per +hash hit, so false_alarms / hash_hits stays bounded no matter how long the +chain is; without the cap that ratio equals the full chain length. We assert +the bound, which is the same integer on every machine. + +The fix is sender-only (it changes no checksum, byte, or protocol field), so +the transferred result must also still be byte-for-byte correct. +""" + +import re + +from rsyncfns import ( + FROMDIR, TODIR, assert_same, rmtree, run_rsync, test_fail, +) + +BLOCK = 256 +NBLOCKS = 8000 # chain length -- much longer than match.c's cap (1024) +RUN = 3000 # length of the constant-byte region in the source +C = 100 # the constant source byte + +# A correct fix bounds the per-hash-hit chain walk to match.c's MAX_CHAIN_LEN +# (1024). Pick an assertion bound comfortably between that cap and the full +# chain length so the test is insensitive to modest cap retuning but still +# fails hard for the unbounded (pre-fix) walk of NBLOCKS per hit. +MAX_FALSE_ALARMS_PER_HIT = NBLOCKS // 4 # 2000: > cap(1024), << chain(8000) + + +def make_decoy_block() -> bytes: + b = bytearray([C]) * BLOCK + # (+1,-2,+1) at positions 0,1,2 preserves get_checksum1 (both s1 and s2) + # while changing the block's content and thus its strong checksum. + b[0] = C + 1 + b[1] = C - 2 + b[2] = C + 1 + return bytes(b) + + +rmtree(FROMDIR) +rmtree(TODIR) +FROMDIR.mkdir(parents=True, exist_ok=True) +TODIR.mkdir(parents=True, exist_ok=True) + +src = FROMDIR / 'image.bin' +dst = TODIR / 'image.bin' + +# Basis: NBLOCKS identical decoys -> one giant weak-checksum chain. +decoy = make_decoy_block() +with open(dst, 'wb') as f: + for _ in range(NBLOCKS): + f.write(decoy) + +# Source: a constant-byte run (constant weak checksum, no strong match). +with open(src, 'wb') as f: + f.write(bytes([C]) * RUN) + +proc = run_rsync('-a', '--no-whole-file', f'--block-size={BLOCK}', + '--no-compress', '--debug=deltasum1', + f'{src}', f'{dst}', capture_output=True) + +out = proc.stdout + proc.stderr +m = re.search(r'hash_hits=(\d+)\s+false_alarms=(\d+)', out) +if not m: + test_fail(f"could not find deltasum stats in rsync output:\n{out}") +hash_hits = int(m.group(1)) +false_alarms = int(m.group(2)) + +if hash_hits == 0: + test_fail("expected the source to hit the decoy chain but hash_hits=0") + +ratio = false_alarms / hash_hits +if ratio > MAX_FALSE_ALARMS_PER_HIT: + test_fail( + f"hash_search() walked ~{ratio:.0f} chain entries per hash hit " + f"(false_alarms={false_alarms}, hash_hits={hash_hits}); the chain of " + f"{NBLOCKS} entries is not being capped -- issue #217 regression") + +# Correctness is non-negotiable: the cap only skips matches, never data. +assert_same(dst, src, label='issue #217 chain-cap transfer') + +print(f"issue #217: bounded at {ratio:.0f} false alarms/hit " + f"(chain={NBLOCKS}, cap keeps it under {MAX_FALSE_ALARMS_PER_HIT})") -- 2.47.3 From 5cb4b8290b28abb0176e5891160aaede667f0ddb Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 12 Jul 2026 13:00:49 +1000 Subject: [PATCH 13/16] syscall: build without AT_SYMLINK_NOFOLLOW --- Makefile.in | 16 +++++++++++----- syscall.c | 17 +++++++++++++++-- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/Makefile.in b/Makefile.in index 5216fb2f..ad024132 100644 --- a/Makefile.in +++ b/Makefile.in @@ -65,6 +65,8 @@ CHECK_SYMLINKS = testsuite/chown-fake_test.py testsuite/devices-fake_test.py \ # Objects for CHECK_PROGS to clean CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o t_chmod_secure.o t_secure_relpath.o trimslash.o wildtest.o +# Compile-only feature-shape checks. +CHECK_COMPILE_OBJS=syscall-no-at-fdcwd.o # note that the -I. is needed to handle config.h when using VPATH .c.o: @@ -72,6 +74,10 @@ CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o t_chmod_se $(CC) -I. -I$(srcdir) $(CFLAGS) $(CPPFLAGS) -c $< @CC_SHOBJ_FLAG@ @OBJ_RESTORE@ +syscall-no-at-fdcwd.o: syscall.c $(HEADERS) + $(CC) -I. -I$(srcdir) $(CFLAGS) $(CPPFLAGS) \ + -DRSYNC_TEST_NO_AT_FDCWD -c $(srcdir)/syscall.c -o $@ + # NOTE: consider running "packaging/smart-make" instead of "make" to auto-handle # any changes to configure.sh and the main Makefile prior to a "make all". all: Makefile rsync$(EXEEXT) stunnel-rsyncd.conf @MAKE_RRSYNC@ @MAKE_MAN@ @@ -300,7 +306,7 @@ rrsync.1: support/rrsync.1.md md-convert Makefile .PHONY: clean clean: cleantests - rm -f *~ $(OBJS) $(CHECK_PROGS) $(CHECK_OBJS) $(CHECK_SYMLINKS) @MAKE_RRSYNC@ \ + rm -f *~ $(OBJS) $(CHECK_PROGS) $(CHECK_OBJS) $(CHECK_COMPILE_OBJS) $(CHECK_SYMLINKS) @MAKE_RRSYNC@ \ git-version.h rounding rounding.h *.old rsync*.1 rsync*.5 @MAKE_RRSYNC_1@ \ *.html daemon-parm.h help-*.h default-*.h proto.h proto.h-tstamp rm -f *.gcno *.gcda lib/*.gcno lib/*.gcda zlib/*.gcno zlib/*.gcda popt/*.gcno popt/*.gcda @@ -378,18 +384,18 @@ COVERAGE_EXCLUDE = -e '(^|/)zlib/' -e '(^|/)popt/' \ # WITHOUT running it. Used by CI jobs that invoke runtests.py directly with # custom options (e.g. the version-mix workflow's --rsync-bin2/--expect-result). .PHONY: check-progs -check-progs: all $(CHECK_PROGS) $(CHECK_SYMLINKS) +check-progs: all $(CHECK_PROGS) $(CHECK_COMPILE_OBJS) $(CHECK_SYMLINKS) .PHONY: check -check: all $(CHECK_PROGS) $(CHECK_SYMLINKS) +check: all $(CHECK_PROGS) $(CHECK_COMPILE_OBJS) $(CHECK_SYMLINKS) $(srcdir)/runtests.py --rsync-bin=`pwd`/rsync$(EXEEXT) -j $(CHECK_J) .PHONY: check29 -check29: all $(CHECK_PROGS) $(CHECK_SYMLINKS) +check29: all $(CHECK_PROGS) $(CHECK_COMPILE_OBJS) $(CHECK_SYMLINKS) $(srcdir)/runtests.py --rsync-bin=`pwd`/rsync$(EXEEXT) -j $(CHECK_J) --protocol=29 .PHONY: check30 -check30: all $(CHECK_PROGS) $(CHECK_SYMLINKS) +check30: all $(CHECK_PROGS) $(CHECK_COMPILE_OBJS) $(CHECK_SYMLINKS) $(srcdir)/runtests.py --rsync-bin=`pwd`/rsync$(EXEEXT) -j $(CHECK_J) --protocol=30 # Whole-suite gcov coverage report (HTML, with branch + decision coverage). diff --git a/syscall.c b/syscall.c index 243f7f24..dfe9ed13 100644 --- a/syscall.c +++ b/syscall.c @@ -22,6 +22,14 @@ #include "rsync.h" +/* Exercise the pre-*at() portability tier on modern build hosts. */ +#ifdef RSYNC_TEST_NO_AT_FDCWD +#undef AT_FDCWD +#undef AT_SYMLINK_NOFOLLOW +#undef HAVE_LINKAT +#undef HAVE_UTIMENSAT +#endif + #if !defined MKNOD_CREATES_SOCKETS && defined HAVE_SYS_UN_H #include #endif @@ -426,7 +434,7 @@ int do_lchown(const char *path, uid_t owner, gid_t group) */ int do_lchown_at(const char *fname, uid_t owner, gid_t group) { -#ifdef AT_FDCWD +#if defined AT_FDCWD && defined AT_SYMLINK_NOFOLLOW extern int am_daemon, am_chrooted; char dirpath[MAXPATHLEN]; const char *bname; @@ -1181,6 +1189,7 @@ static int do_xstat_at(const char *path, STRUCT_STAT *st, int at_flags, int (*fa errno = e; return ret; #else + (void)at_flags; return fallback(path, st); #endif } @@ -1192,8 +1201,10 @@ int do_stat_at(const char *path, STRUCT_STAT *st) int do_lstat_at(const char *path, STRUCT_STAT *st) { -#ifdef SUPPORT_LINKS +#if defined SUPPORT_LINKS && defined AT_FDCWD && defined AT_SYMLINK_NOFOLLOW return do_xstat_at(path, st, AT_SYMLINK_NOFOLLOW, do_lstat); +#elif defined SUPPORT_LINKS + return do_lstat(path, st); #else return do_xstat_at(path, st, 0, do_stat); #endif @@ -2007,6 +2018,7 @@ cleanup: #endif // O_NOFOLLOW, O_DIRECTORY } +#if defined O_NOFOLLOW && defined O_DIRECTORY && defined AT_FDCWD /* Fill buf with len random bytes. Prefers /dev/urandom for cryptographic * quality; falls back to rand() if /dev/urandom cannot be opened or read * (e.g. inside a chroot or container without /dev populated). */ @@ -2027,6 +2039,7 @@ static void rand_bytes(unsigned char *buf, size_t len) buf[i] = (unsigned char)rand(); } } +#endif /* Secure version of mkstemp that prevents symlink attacks on parent directories. -- 2.47.3 From 3c9a12011e88eb30a3b23211cbb8fcc261030673 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 12 Jul 2026 13:11:43 +1000 Subject: [PATCH 14/16] ci: fix no-AT_FDCWD compile check --- syscall.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/syscall.c b/syscall.c index dfe9ed13..c7f12d2c 100644 --- a/syscall.c +++ b/syscall.c @@ -27,7 +27,9 @@ #undef AT_FDCWD #undef AT_SYMLINK_NOFOLLOW #undef HAVE_LINKAT +#undef HAVE_OPENAT2 #undef HAVE_UTIMENSAT +#undef O_RESOLVE_BENEATH #endif #if !defined MKNOD_CREATES_SOCKETS && defined HAVE_SYS_UN_H -- 2.47.3 From 9062840a9165c38956f0235ae0b72e75bb2149a3 Mon Sep 17 00:00:00 2001 From: Zen Dodd Date: Sun, 12 Jul 2026 13:17:34 +1000 Subject: [PATCH 15/16] ci: restore default build target --- Makefile.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.in b/Makefile.in index ad024132..ac14877b 100644 --- a/Makefile.in +++ b/Makefile.in @@ -74,15 +74,15 @@ CHECK_COMPILE_OBJS=syscall-no-at-fdcwd.o $(CC) -I. -I$(srcdir) $(CFLAGS) $(CPPFLAGS) -c $< @CC_SHOBJ_FLAG@ @OBJ_RESTORE@ -syscall-no-at-fdcwd.o: syscall.c $(HEADERS) - $(CC) -I. -I$(srcdir) $(CFLAGS) $(CPPFLAGS) \ - -DRSYNC_TEST_NO_AT_FDCWD -c $(srcdir)/syscall.c -o $@ - # NOTE: consider running "packaging/smart-make" instead of "make" to auto-handle # any changes to configure.sh and the main Makefile prior to a "make all". all: Makefile rsync$(EXEEXT) stunnel-rsyncd.conf @MAKE_RRSYNC@ @MAKE_MAN@ .PHONY: all +syscall-no-at-fdcwd.o: syscall.c $(HEADERS) + $(CC) -I. -I$(srcdir) $(CFLAGS) $(CPPFLAGS) \ + -DRSYNC_TEST_NO_AT_FDCWD -c $(srcdir)/syscall.c -o $@ + .PHONY: install install: all -$(MKDIR_P) $(DESTDIR)$(bindir) -- 2.47.3 From 3b84610ccb35d1355f6e88e0e870bef48a164fbb Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 20 Jul 2026 12:53:16 +1000 Subject: [PATCH 16/16] manpage: clarify remote-shell daemon user@ handling The description of user@host::module transfers over a remote shell only documented the "ssh -l ssh-user" form, which led readers to conclude that user@ never reaches the remote shell. In fact, for the simple `--rsh=ssh user@host::module` form the user@ prefix is used both as the ssh login user (ssh -l user) and as the rsync-user offered to the module; the two are the same name. rsync only omits its own -l when the remote shell command already specifies one, in which case user@ becomes the rsync-user alone. Spell out the default behaviour and why the explicit -l is needed to use a different ssh login than the rsync-user. --- rsync.1.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/rsync.1.md b/rsync.1.md index fdf0b2e9..2d38efe3 100644 --- a/rsync.1.md +++ b/rsync.1.md @@ -365,16 +365,26 @@ RSYNC_RSH in the environment will not turn on this functionality.) For example: > rsync -av --rsh=ssh host::module /dest -If you need to specify a different remote-shell user, keep in mind that the -user@ prefix in front of the host is specifying the rsync-user value (for a -module that requires user-based authentication). This means that you must give -the '-l user' option to ssh when specifying the remote-shell, as in this -example that uses the short version of the [`--rsh`](#opt) option: +A "user@" prefix in front of the host serves two purposes at once in this mode: +it is the rsync-user value (used to log in to a module that requires user-based +authentication) and, by default, it is also passed to the remote shell as the +login user. So the simple form + +> rsync -av --rsh=ssh user@host::module /dest + +runs `ssh -l user host` and offers "user" as the rsync-user to the module; the +two are forced to be the same name. + +If you need the remote-shell (ssh) login to use a different name than the +rsync-user, give an explicit '-l' option to ssh in the remote-shell command. +When ssh is already told which user to log in as, rsync does not add its own +'-l', so the "user@" prefix is then used only as the rsync-user. For example, +using the short version of the [`--rsh`](#opt) option: > rsync -av -e "ssh -l ssh-user" rsync-user@host::module /dest -The "ssh-user" will be used at the ssh level; the "rsync-user" will be used to -log-in to the "module". +Here "ssh-user" is used at the ssh level while "rsync-user" is used to log in to +the "module". In this setup, the daemon is started by the ssh command that is accessing the system (which can be forced via the `~/.ssh/authorized_keys` file, if desired). -- 2.47.3