mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-09-12 21:28:25 -04:00
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.
This commit is contained in:
1 parent
abcf37a1d3
commit
acfc94ef48
1 file changed
+26
-2
@@ -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
|
* 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
|
* ours runs, so a plain "resolved in the constructor" pointer is still NULL and
|
||||||
* the process dies with SIGSEGV inside the loader. */
|
* 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)
|
static void hook_resolve(void)
|
||||||
{
|
{
|
||||||
|
if (hook_resolving)
|
||||||
|
return;
|
||||||
|
hook_resolving = 1;
|
||||||
if (!real_open) real_open = dlsym(RTLD_NEXT, "open");
|
if (!real_open) real_open = dlsym(RTLD_NEXT, "open");
|
||||||
if (!real_openat) real_openat = dlsym(RTLD_NEXT, "openat");
|
if (!real_openat) real_openat = dlsym(RTLD_NEXT, "openat");
|
||||||
if (!real_fstatat) real_fstatat = dlsym(RTLD_NEXT, "fstatat");
|
if (!real_fstatat) real_fstatat = dlsym(RTLD_NEXT, "fstatat");
|
||||||
if (!real_fxstatat) real_fxstatat = dlsym(RTLD_NEXT, "__fxstatat");
|
if (!real_fxstatat) real_fxstatat = dlsym(RTLD_NEXT, "__fxstatat");
|
||||||
|
hook_resolving = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Last-resort passthrough if even dlsym() is unusable this early. openat(2)
|
/* 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 */
|
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)
|
static int is_victim_write(const char *path, int flags)
|
||||||
{
|
{
|
||||||
return path && strcmp(path, "victim") == 0
|
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");
|
partial = getenv("RSYNC_PARTIAL_RETRY_DIR");
|
||||||
secret = getenv("RSYNC_PARTIAL_RETRY_SECRET");
|
secret = getenv("RSYNC_PARTIAL_RETRY_SECRET");
|
||||||
|
|
||||||
if (flags & O_CREAT) {
|
if (HOOK_TAKES_MODE(flags)) {
|
||||||
va_list ap;
|
va_list ap;
|
||||||
va_start(ap, flags);
|
va_start(ap, flags);
|
||||||
mode = (mode_t)va_arg(ap, int);
|
mode = (mode_t)va_arg(ap, int);
|
||||||
@@ -187,7 +203,7 @@ int open(const char *path, int flags, ...)
|
|||||||
|
|
||||||
hook_resolve();
|
hook_resolve();
|
||||||
|
|
||||||
if (flags & O_CREAT) {
|
if (HOOK_TAKES_MODE(flags)) {
|
||||||
va_list ap;
|
va_list ap;
|
||||||
va_start(ap, flags);
|
va_start(ap, flags);
|
||||||
mode = (mode_t)va_arg(ap, int);
|
mode = (mode_t)va_arg(ap, int);
|
||||||
@@ -331,6 +347,14 @@ def run():
|
|||||||
held.rename(partial)
|
held.rename(partial)
|
||||||
|
|
||||||
ctx = f'rc={result.returncode}, output={result.stdout!r}'
|
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():
|
if not markers['load'].exists():
|
||||||
test_skipped(f'LD_PRELOAD hook was not loaded ({ctx})')
|
test_skipped(f'LD_PRELOAD hook was not loaded ({ctx})')
|
||||||
if not markers['eacces'].exists():
|
if not markers['eacces'].exists():
|
||||||
|
|||||||
Reference in new issue
Block a user