From 6fad1d7d740a5de1be8474d65dd65b6aee39d45c Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Sat, 6 Jun 2026 18:43:06 +1000 Subject: [PATCH] testsuite,ci: mark recv-discard-nullderef CI skip and tighten its check The regression test honestly skips when it cannot force the receiver's output mkstemp() to fail -- as root (root bypasses DAC) and on Cygwin (chmod 0555 does not deny the owner a write). The ubuntu, ubuntu-22.04, almalinux and macOS jobs run `make check` as root, and Cygwin can't enforce the unwritable directory, so the test skips on all of them. runtests.py fails a run on any skip-set mismatch, so add the test to those jobs' RSYNC_EXPECT_SKIPPED lists; the BSD/Solaris jobs run as root too but enforce no expected-skip set, so they need no change. Also tighten the pass condition. The post-chmod writability probe already guarantees the receiver discards (mkstemp must fail), so an exit 0 would mean the file actually transferred and the discard path was never exercised -- a silent false-pass. Require exactly exit 23 (the forced discard leaves the file untransferred); 12 remains the pre-fix crash. --- .github/workflows/almalinux-8-build.yml | 2 +- .github/workflows/cygwin-build.yml | 2 +- .github/workflows/macos-build.yml | 2 +- .github/workflows/ubuntu-22.04-build.yml | 6 +++--- .github/workflows/ubuntu-build.yml | 6 +++--- testsuite/recv-discard-nullderef_test.py | 23 +++++++++++++---------- 6 files changed, 22 insertions(+), 19 deletions(-) diff --git a/.github/workflows/almalinux-8-build.yml b/.github/workflows/almalinux-8-build.yml index 1a0b0e15..f3ba969f 100644 --- a/.github/workflows/almalinux-8-build.yml +++ b/.github/workflows/almalinux-8-build.yml @@ -62,7 +62,7 @@ jobs: # crtimes-not-supported skip matches the other Linux jobs; # daemon-chroot-acl and proxy-response-line-too-long skip because # the default (secure) transport opens no listening socket. - run: RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check + run: RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check - name: check (TCP daemon transport) # Second run exercising the real loopback-TCP daemon path. run: ./runtests.py --rsync-bin="$PWD/rsync" --use-tcp -j 8 diff --git a/.github/workflows/cygwin-build.yml b/.github/workflows/cygwin-build.yml index a63a3f26..a14e9d06 100644 --- a/.github/workflows/cygwin-build.yml +++ b/.github/workflows/cygwin-build.yml @@ -46,7 +46,7 @@ jobs: # RESOLVE_BENEATH symlink-race tests. symlink-dirlink-basis also now # RUNS (the #915 non-daemon basis open uses a plain do_open, restoring # following an in-tree dir-symlink basis without RESOLVE_BENEATH). - run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,acls,bare-do-open-symlink-race,chdir-symlink-race,chown,daemon-access-ip,daemon-chroot-acl,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,sender-flist-symlink-leak,simd-checksum make check' + run: bash -c 'RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,acls,bare-do-open-symlink-race,chdir-symlink-race,chown,daemon-access-ip,daemon-chroot-acl,devices,dir-sgid,open-noatime,protected-regular,proxy-response-line-too-long,recv-discard-nullderef,sender-flist-symlink-leak,simd-checksum make check' - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. diff --git a/.github/workflows/macos-build.yml b/.github/workflows/macos-build.yml index 067671ce..971389fb 100644 --- a/.github/workflows/macos-build.yml +++ b/.github/workflows/macos-build.yml @@ -44,7 +44,7 @@ jobs: # chown-fake / devices-fake / xattrs / xattrs-hlink now RUN on macOS # (rsyncfns.py drives xattrs via the `xattr` command), verified on a # real macOS host, so they're no longer in the skip set. - run: sudo RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,chmod-temp-dir,daemon-access-ip,daemon-chroot-acl,dir-sgid,open-noatime,preallocate,protected-regular,proxy-response-line-too-long,simd-checksum,sparse make check + run: sudo RSYNC_EXPECT_SKIPPED=acls-default,acls-depth,chmod-temp-dir,daemon-access-ip,daemon-chroot-acl,dir-sgid,open-noatime,preallocate,protected-regular,proxy-response-line-too-long,recv-discard-nullderef,simd-checksum,sparse make check - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. diff --git a/.github/workflows/ubuntu-22.04-build.yml b/.github/workflows/ubuntu-22.04-build.yml index 392b5e73..af14ce4b 100644 --- a/.github/workflows/ubuntu-22.04-build.yml +++ b/.github/workflows/ubuntu-22.04-build.yml @@ -39,11 +39,11 @@ jobs: - name: info run: rsync --version - name: check - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check - name: check30 - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check30 + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check30 - name: check29 - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check29 + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check29 - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd; the default # 'make check' above uses the secure stdio-pipe transport. diff --git a/.github/workflows/ubuntu-build.yml b/.github/workflows/ubuntu-build.yml index c142d693..72f19e3f 100644 --- a/.github/workflows/ubuntu-build.yml +++ b/.github/workflows/ubuntu-build.yml @@ -63,11 +63,11 @@ jobs: - name: info run: rsync --version - name: check - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check - name: check30 - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check30 + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check30 - name: check29 - run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check29 + run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check29 - name: check (TCP daemon transport) # Second run with daemon tests over a real loopback rsyncd. The default # 'make check' above uses the secure stdio-pipe transport (no listening diff --git a/testsuite/recv-discard-nullderef_test.py b/testsuite/recv-discard-nullderef_test.py index a0919578..e130203f 100755 --- a/testsuite/recv-discard-nullderef_test.py +++ b/testsuite/recv-discard-nullderef_test.py @@ -104,20 +104,23 @@ finally: rc = proc.returncode # A receiver SIGSEGV manifests to the client as a protocol error (the daemon's -# receiver child crashes mid-stream and the connection drops). Pre-fix this is -# code 12 (error in rsync protocol data stream); post-fix the receiver drains -# the delta and reports a benign "could not transfer" (code 23), or succeeds. +# receiver child crashes mid-stream and the connection drops): exit code 12. +# With the fix the receiver drains the delta and, because the forced-unwritable +# destination leaves the file untransferred, the run reports the benign "some +# files were not transferred" -- exit code 23. # -# rsync's own exit codes are all < 128, so we can't read the receiver's signal -# directly from the client. The discriminator is the PROTOCOL error: only a -# crashed (or otherwise vanished) receiver produces code 12 here. A clean -# discard yields 23 (file not transferred) or 0. +# 23 is the ONLY non-crash outcome here: the writability probe above guarantees +# the receiver's mkstemp() fails, so the file is always discarded. An exit 0 +# would mean the file actually transferred -- the discard path was NOT exercised +# and the run proves nothing -- so require exactly 23 (and call out 12 as the +# pre-fix crash). if rc == 12: test_fail(f"receiver crashed on the discard path (rsync exited {rc}: " "error in rsync protocol data stream -- the receiver child " "SIGSEGV'd in full_fname(NULL))") -if rc not in (0, 23): - test_fail(f"unexpected rsync exit {rc} (expected 0 or 23, a benign " - "discard; 12 would be the crash)") +if rc != 23: + test_fail(f"expected rsync exit 23 (the forced discard leaves the file " + f"untransferred); got {rc} -- the discard path was not exercised, " + "so this run validates nothing (12 would be the pre-fix crash)") print(f"OK: receiver discarded the delta without crashing (rsync exit {rc})") -- 2.47.3