Skip to content

Commit 6fad1d7

Browse files
committed
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.
1 parent b8562db commit 6fad1d7

6 files changed

Lines changed: 22 additions & 19 deletions

File tree

.github/workflows/almalinux-8-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ jobs:
6262
# crtimes-not-supported skip matches the other Linux jobs;
6363
# daemon-chroot-acl and proxy-response-line-too-long skip because
6464
# the default (secure) transport opens no listening socket.
65-
run: RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check
65+
run: RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check
6666
- name: check (TCP daemon transport)
6767
# Second run exercising the real loopback-TCP daemon path.
6868
run: ./runtests.py --rsync-bin="$PWD/rsync" --use-tcp -j 8

.github/workflows/cygwin-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ jobs:
4646
# RESOLVE_BENEATH symlink-race tests. symlink-dirlink-basis also now
4747
# RUNS (the #915 non-daemon basis open uses a plain do_open, restoring
4848
# following an in-tree dir-symlink basis without RESOLVE_BENEATH).
49-
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'
49+
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'
5050
- name: check (TCP daemon transport)
5151
# Second run with daemon tests over a real loopback rsyncd; the default
5252
# 'make check' above uses the secure stdio-pipe transport.

.github/workflows/macos-build.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ jobs:
4444
# chown-fake / devices-fake / xattrs / xattrs-hlink now RUN on macOS
4545
# (rsyncfns.py drives xattrs via the `xattr` command), verified on a
4646
# real macOS host, so they're no longer in the skip set.
47-
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
47+
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
4848
- name: check (TCP daemon transport)
4949
# Second run with daemon tests over a real loopback rsyncd; the default
5050
# 'make check' above uses the secure stdio-pipe transport.

.github/workflows/ubuntu-22.04-build.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ jobs:
3939
- name: info
4040
run: rsync --version
4141
- name: check
42-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check
42+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check
4343
- name: check30
44-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check30
44+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check30
4545
- name: check29
46-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check29
46+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check29
4747
- name: check (TCP daemon transport)
4848
# Second run with daemon tests over a real loopback rsyncd; the default
4949
# 'make check' above uses the secure stdio-pipe transport.

.github/workflows/ubuntu-build.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ jobs:
6363
- name: info
6464
run: rsync --version
6565
- name: check
66-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check
66+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check
6767
- name: check30
68-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check30
68+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check30
6969
- name: check29
70-
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long make check29
70+
run: sudo RSYNC_EXPECT_SKIPPED=crtimes,daemon-access-ip,daemon-chroot-acl,proxy-response-line-too-long,recv-discard-nullderef make check29
7171
- name: check (TCP daemon transport)
7272
# Second run with daemon tests over a real loopback rsyncd. The default
7373
# 'make check' above uses the secure stdio-pipe transport (no listening

testsuite/recv-discard-nullderef_test.py

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -104,20 +104,23 @@
104104
rc = proc.returncode
105105

106106
# A receiver SIGSEGV manifests to the client as a protocol error (the daemon's
107-
# receiver child crashes mid-stream and the connection drops). Pre-fix this is
108-
# code 12 (error in rsync protocol data stream); post-fix the receiver drains
109-
# the delta and reports a benign "could not transfer" (code 23), or succeeds.
107+
# receiver child crashes mid-stream and the connection drops): exit code 12.
108+
# With the fix the receiver drains the delta and, because the forced-unwritable
109+
# destination leaves the file untransferred, the run reports the benign "some
110+
# files were not transferred" -- exit code 23.
110111
#
111-
# rsync's own exit codes are all < 128, so we can't read the receiver's signal
112-
# directly from the client. The discriminator is the PROTOCOL error: only a
113-
# crashed (or otherwise vanished) receiver produces code 12 here. A clean
114-
# discard yields 23 (file not transferred) or 0.
112+
# 23 is the ONLY non-crash outcome here: the writability probe above guarantees
113+
# the receiver's mkstemp() fails, so the file is always discarded. An exit 0
114+
# would mean the file actually transferred -- the discard path was NOT exercised
115+
# and the run proves nothing -- so require exactly 23 (and call out 12 as the
116+
# pre-fix crash).
115117
if rc == 12:
116118
test_fail(f"receiver crashed on the discard path (rsync exited {rc}: "
117119
"error in rsync protocol data stream -- the receiver child "
118120
"SIGSEGV'd in full_fname(NULL))")
119-
if rc not in (0, 23):
120-
test_fail(f"unexpected rsync exit {rc} (expected 0 or 23, a benign "
121-
"discard; 12 would be the crash)")
121+
if rc != 23:
122+
test_fail(f"expected rsync exit 23 (the forced discard leaves the file "
123+
f"untransferred); got {rc} -- the discard path was not exercised, "
124+
"so this run validates nothing (12 would be the pre-fix crash)")
122125

123126
print(f"OK: receiver discarded the delta without crashing (rsync exit {rc})")

0 commit comments

Comments
 (0)