From c88d4e212338938155932bca73164d02e6151d8a Mon Sep 17 00:00:00 2001 From: Peter Korolev Date: Tue, 13 Oct 2026 11:04:11 +0000 Subject: [PATCH 1/2] cleanup: don't flush the write buffer from a signal handler When SIGUSR1 aborts an --inplace/--append transfer, _exit_cleanup() runs inside the handler and flushes the receiver's write buffer. If the handler interrupted flush_write_file() after write() returned but before wf_writeBufCnt was updated, the same bytes are written a second time, and with --append the file keeps a duplicated 256 KiB region. Skip that flush when called from a signal handler. A SIGINT, SIGTERM or SIGHUP that reaches the receiver itself still exits from normal code through got_kill_signal, so its flush is unaffected. --- cleanup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cleanup.c b/cleanup.c index 7f1864ccb..38aafb3d0 100644 --- a/cleanup.c +++ b/cleanup.c @@ -162,7 +162,8 @@ NORETURN void _exit_cleanup(int code, const char *file, int line) cleanup_fd_r = -1; } if (cleanup_fd_w != -1) { - flush_write_file(cleanup_fd_w); + if (!called_from_signal_handler) + flush_write_file(cleanup_fd_w); close(cleanup_fd_w); cleanup_fd_w = -1; } From a9b52dfe38cb7779147715572b20e9d576f8adb1 Mon Sep 17 00:00:00 2001 From: Peter Korolev Date: Tue, 13 Oct 2026 11:22:47 +0000 Subject: [PATCH 2/2] testsuite: abort --append/--inplace in the middle of a write Hold the receiver right after its first write() to the destination, with every signal pending, send it SIGUSR1 and check that the destination is still a prefix of the source. This fails without the previous commit. The hold is an LD_PRELOAD shim, so the test is Linux-only and is expected to skip on macOS and Cygwin. --- testsuite/append-abort-mid-write_test.py | 265 +++++++++++++++++++++++ testsuite/skiplist/cygwin.txt | 1 + testsuite/skiplist/macos.txt | 1 + 3 files changed, 267 insertions(+) create mode 100644 testsuite/append-abort-mid-write_test.py diff --git a/testsuite/append-abort-mid-write_test.py b/testsuite/append-abort-mid-write_test.py new file mode 100644 index 000000000..b0b1ebe94 --- /dev/null +++ b/testsuite/append-abort-mid-write_test.py @@ -0,0 +1,265 @@ +#!/usr/bin/env python3 +# An --append or --inplace transfer that is aborted while the receiver is +# writing out its buffer must leave the destination a clean prefix of the +# source. +# +# When a transfer aborts, the generator sends the receiver SIGUSR1, and the +# handler runs exit_cleanup(). For these modes that cleanup used to flush the +# receiver's write buffer. If the signal was handled after a write() of the +# buffer had returned, but before flush_write_file() had counted those bytes as +# written, the cleanup wrote them a second time. With --append the duplicate +# then stayed in the file for good, since later runs only add data past the +# end. +# +# A signal that arrives during a slow write (FUSE, NFS) is handled exactly +# there, as the write returns. To reach that point deterministically, this +# test runs rsync under a scratch-built LD_PRELOAD shim: right after the +# receiver's first large write() to the destination file, the shim keeps every +# signal pending and reports the receiver's pid, and the test sends SIGUSR1 +# before letting it continue. + +import os +import shlex +import shutil +import signal +import subprocess +import sys +import time + +from rsyncfns import ( + RSYNC, SCRATCHDIR, assert_same, make_data_file, makepath, rmtree, + rsync_argv, run_rsync, split_rsync_cmd, test_fail, test_skipped, +) + + +SHIM_C = r''' +#define _GNU_SOURCE 1 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +static ssize_t (*real_write)(int, const void *, size_t); +static char target_path[PATH_MAX]; +static int held; + +static void note(const char *env, const char *text) +{ + const char *path = getenv(env); + int fd; + + if (!path || !*path) + return; + fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (fd >= 0) { + (void)real_write(fd, text, strlen(text)); + (void)close(fd); + } +} + +__attribute__((constructor)) +static void init(void) +{ + const char *target = getenv("RSYNC_MIDWRITE_TARGET"); + + real_write = (ssize_t (*)(int, const void *, size_t)) + dlsym(RTLD_NEXT, "write"); + if (target && *target && !realpath(target, target_path)) { + strncpy(target_path, target, sizeof target_path - 1); + target_path[sizeof target_path - 1] = '\0'; + } + note("RSYNC_MIDWRITE_LOADED", "loaded\n"); +} + +static int fd_is_target(int fd) +{ + char procpath[64]; + char actual[PATH_MAX]; + ssize_t n; + + if (!target_path[0]) + return 0; + snprintf(procpath, sizeof procpath, "/proc/self/fd/%d", fd); + n = readlink(procpath, actual, sizeof actual - 1); + if (n < 0) + return 0; + actual[n] = '\0'; + return strcmp(actual, target_path) == 0; +} + +/* Report our pid, then wait (at most a minute) for the test to release us. */ +static void hold(void) +{ + const char *release = getenv("RSYNC_MIDWRITE_RELEASE"); + struct timespec tick = { 0, 10 * 1000 * 1000 }; + char pid[32]; + int i; + + snprintf(pid, sizeof pid, "%d\n", (int)getpid()); + note("RSYNC_MIDWRITE_HELD", pid); + for (i = 0; release && i < 6000 && access(release, F_OK) != 0; i++) + nanosleep(&tick, NULL); +} + +ssize_t write(int fd, const void *buf, size_t count) +{ + sigset_t all, old; + ssize_t ret; + int saved_errno; + + if (!real_write) { + errno = EIO; + return -1; + } + if (held || count < 65536 || !fd_is_target(fd)) + return real_write(fd, buf, count); + + held = 1; + sigfillset(&all); + sigprocmask(SIG_BLOCK, &all, &old); + ret = real_write(fd, buf, count); + saved_errno = errno; + hold(); + sigprocmask(SIG_SETMASK, &old, NULL); + errno = saved_errno; + return ret; +} +''' + + +def compiler(): + cc = os.environ.get('CC') + if cc: + return shlex.split(cc) + for name in ('cc', 'gcc', 'clang'): + path = shutil.which(name) + if path: + return [path] + return None + + +def build_shim(): + if not sys.platform.startswith('linux'): + test_skipped('the mid-write hook is an LD_PRELOAD shim that matches ' + 'the destination fd through /proc/self/fd') + cc = compiler() + if not cc: + test_skipped('no C compiler available to build the mid-write shim') + + cfile = SCRATCHDIR / 'midwrite.c' + sofile = SCRATCHDIR / 'midwrite.so' + cfile.write_text(SHIM_C) + proc = subprocess.run( + cc + ['-shared', '-fPIC', '-O2', '-Wall', '-Wextra', + '-o', str(sofile), str(cfile), '-ldl'], + stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True) + if proc.returncode != 0 or not sofile.is_file(): + test_skipped('could not build the mid-write shim:\n' + + (proc.stdout or '').strip()) + return sofile + + +def gone(pid): + """True once `pid` has exited (a zombie counts: it can no longer write).""" + try: + with open(f'/proc/{pid}/stat') as f: + return f.read().rsplit(')', 1)[1].split()[0] == 'Z' + except FileNotFoundError: + return True + + +def first_difference(a, b): + for i in range(0, min(len(a), len(b)), 4096): + if a[i:i + 4096] != b[i:i + 4096]: + return next(j for j in range(i, i + 4096) if a[j] != b[j]) + return min(len(a), len(b)) + + +def abort_mid_write(mode): + """Abort an rsync `mode` transfer while the receiver is inside its first + write() to the destination, then check what that left behind.""" + base = SCRATCHDIR / 'append-abort' + src = base / 'src' + dest = base / 'dest' + rmtree(base) + makepath(src, dest) + + # A growing file whose first MiB was synced by an earlier run. + make_data_file(src / 'grow.bin', 4 * 1024 * 1024) + data = (src / 'grow.bin').read_bytes() + prefix = 1024 * 1024 + (dest / 'grow.bin').write_bytes(data[:prefix]) + + loaded = SCRATCHDIR / 'midwrite.loaded' + held = SCRATCHDIR / 'midwrite.held' + release = SCRATCHDIR / 'midwrite.release' + for p in (loaded, held, release): + p.unlink(missing_ok=True) + env = os.environ.copy() + env['LD_PRELOAD'] = str(shim) + env['RSYNC_MIDWRITE_TARGET'] = str((dest / 'grow.bin').resolve()) + env['RSYNC_MIDWRITE_LOADED'] = str(loaded) + env['RSYNC_MIDWRITE_HELD'] = str(held) + env['RSYNC_MIDWRITE_RELEASE'] = str(release) + + argv = rsync_argv('-a', mode, f'{src}/', f'{dest}/') + print('Running:', ' '.join(argv)) + proc = subprocess.Popen(argv, env=env) + try: + deadline = time.monotonic() + 60 + while not held.is_file() or not held.read_text().endswith('\n'): + if proc.poll() is not None or time.monotonic() > deadline: + break + time.sleep(0.01) + if not loaded.exists(): + test_skipped('mid-write shim was not loaded; rsync may be static ' + 'or the platform ignored LD_PRELOAD') + if not held.is_file(): + test_fail(f'{mode}: the receiver never wrote to the destination ' + 'file, so the shim could not hold it') + receiver = int(held.read_text()) + # What the generator's exit cleanup does when a transfer aborts. + os.kill(receiver, signal.SIGUSR1) + finally: + release.touch() + try: + proc.wait(timeout=60) + except subprocess.TimeoutExpired: + proc.kill() + test_fail(f'{mode}: rsync did not exit after its receiver was aborted') + deadline = time.monotonic() + 30 + while not gone(receiver): + if time.monotonic() > deadline: + test_fail(f'{mode}: the receiver did not exit after SIGUSR1') + time.sleep(0.01) + if proc.returncode == 0: + test_fail(f'{mode}: rsync exited 0 although its receiver was aborted') + + got = (dest / 'grow.bin').read_bytes() + if len(got) > len(data) or got != data[:len(got)]: + test_fail(f'{mode}: after the abort the destination ({len(got)} ' + f'bytes) is not a prefix of the source; it differs at byte ' + f'{first_difference(got, data)}, so the receiver wrote data ' + 'it had already written') + + run_rsync('-a', mode, f'{src}/', f'{dest}/') + assert_same(src / 'grow.bin', dest / 'grow.bin', + label=f'{mode} rerun after the abort') + + +if os.path.basename(split_rsync_cmd(RSYNC)[0]) == 'valgrind': + test_skipped('the LD_PRELOAD write hook is not meaningful under valgrind') + +shim = build_shim() +for mode in ('--append', '--inplace'): + abort_mid_write(mode) + +print('append-abort-mid-write: an abort in the middle of a write left the ' + 'destination a clean prefix for --append and --inplace') diff --git a/testsuite/skiplist/cygwin.txt b/testsuite/skiplist/cygwin.txt index 3ea71beb5..3e3aa4bb2 100644 --- a/testsuite/skiplist/cygwin.txt +++ b/testsuite/skiplist/cygwin.txt @@ -10,6 +10,7 @@ acls acls-default acls-depth acls-unpinnable +append-abort-mid-write # the mid-write hook is an LD_PRELOAD, Linux-only backup-acl-xattr-cache backup-dir-symlink-race basis-xname-traversal diff --git a/testsuite/skiplist/macos.txt b/testsuite/skiplist/macos.txt index 7ba8d589d..ac8658fdc 100644 --- a/testsuite/skiplist/macos.txt +++ b/testsuite/skiplist/macos.txt @@ -9,6 +9,7 @@ acl-symlink-race acls-default acls-depth acls-unpinnable +append-abort-mid-write # the mid-write hook is an LD_PRELOAD, Linux-only backup-acl-xattr-cache backup-crossdev-copy chmod-temp-dir