From acfc94ef48b18ea1388cf6fe49feff8127544625 Mon Sep 17 00:00:00 2001
From: Andrew Tridgell <andrew@tridgell.net>
Date: Sun, 26 Jul 2026 16:10:44 +1000
Subject: [PATCH] testsuite: harden the LD_PRELOAD hook and stop a crash
 reading as a skip

Follow-ups from review of the lazy-resolution fix:

- Guard resolution against re-entry.  If dlsym() ever reaches an
  interposed function the nested wrapper would recurse; it now takes the
  raw path instead.

- Forward a mode for O_TMPFILE as well as O_CREAT.  rsync itself never
  uses it, but the hook interposes every library in the process.  The
  test is an equality one because Linux defines O_TMPFILE as
  __O_TMPFILE|O_DIRECTORY, so a plain & would also match O_DIRECTORY.

- Treat death by signal as a failure rather than a skip.  The load marker
  is written by the hook, so a crash before that point is
  indistinguishable from the hook never loading -- which is precisely how
  the AlmaLinux SIGSEGV stayed hidden.

Note the signal branch is not exercised by any current configuration:
with the raw openat(2) fallback in place the hook no longer crashes even
when resolution fails, which is why reconstructing the pre-fix behaviour
does not reproduce it.
---
 ...tial-protected-regular-retry-linux_test.py | 28 +++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/testsuite/partial-protected-regular-retry-linux_test.py b/testsuite/partial-protected-regular-retry-linux_test.py
index 0ee91725..9a4420fa 100644
--- a/testsuite/partial-protected-regular-retry-linux_test.py
+++ b/testsuite/partial-protected-regular-retry-linux_test.py
@@ -64,12 +64,20 @@ static int (*real_fxstatat)(int, int, const char *, struct stat *, int);
  * AlmaLinux 8, OPENSSL_init_library() calls open() from its constructor before
  * ours runs, so a plain "resolved in the constructor" pointer is still NULL and
  * the process dies with SIGSEGV inside the loader. */
+/* If dlsym() itself reaches an interposed function, the nested wrapper must
+ * not recurse back into resolution -- it takes the raw path instead. */
+static __thread int hook_resolving;
+
 static void hook_resolve(void)
 {
+    if (hook_resolving)
+        return;
+    hook_resolving = 1;
     if (!real_open)     real_open     = dlsym(RTLD_NEXT, "open");
     if (!real_openat)   real_openat   = dlsym(RTLD_NEXT, "openat");
     if (!real_fstatat)  real_fstatat  = dlsym(RTLD_NEXT, "fstatat");
     if (!real_fxstatat) real_fxstatat = dlsym(RTLD_NEXT, "__fxstatat");
+    hook_resolving = 0;
 }
 
 /* Last-resort passthrough if even dlsym() is unusable this early.  openat(2)
@@ -135,6 +143,14 @@ static int swap_and_deny(void)
     return 0;   /* caller falls through to the real call */
 }
 
+#ifdef O_TMPFILE
+/* O_TMPFILE is (__O_TMPFILE | O_DIRECTORY) on Linux, so a plain & test would
+ * also match an ordinary O_DIRECTORY open. */
+# define HOOK_TAKES_MODE(f) (((f) & O_CREAT) || (((f) & O_TMPFILE) == O_TMPFILE))
+#else
+# define HOOK_TAKES_MODE(f) ((f) & O_CREAT)
+#endif
+
 static int is_victim_write(const char *path, int flags)
 {
     return path && strcmp(path, "victim") == 0
@@ -151,7 +167,7 @@ int openat(int dfd, const char *path, int flags, ...)
     partial = getenv("RSYNC_PARTIAL_RETRY_DIR");
     secret = getenv("RSYNC_PARTIAL_RETRY_SECRET");
 
-    if (flags & O_CREAT) {
+    if (HOOK_TAKES_MODE(flags)) {
         va_list ap;
         va_start(ap, flags);
         mode = (mode_t)va_arg(ap, int);
@@ -187,7 +203,7 @@ int open(const char *path, int flags, ...)
 
     hook_resolve();
 
-    if (flags & O_CREAT) {
+    if (HOOK_TAKES_MODE(flags)) {
         va_list ap;
         va_start(ap, flags);
         mode = (mode_t)va_arg(ap, int);
@@ -331,6 +347,14 @@ def run():
         held.rename(partial)
 
     ctx = f'rc={result.returncode}, output={result.stdout!r}'
+    # A negative returncode is death by signal.  That is never a reason to skip:
+    # the hook killing the process under test is a bug in the hook, and reporting
+    # it as "not loaded" is exactly how a SIGSEGV on AlmaLinux 8 hid for weeks --
+    # the marker is written by the hook, so a crash before that looks identical
+    # to the hook never having loaded.
+    if result.returncode is not None and result.returncode < 0:
+        test_fail(f'rsync died by signal {-result.returncode} under the '
+                  f'LD_PRELOAD hook ({ctx})')
     if not markers['load'].exists():
         test_skipped(f'LD_PRELOAD hook was not loaded ({ctx})')
     if not markers['eacces'].exists():