Commit ed047e3d authored by Ramon Nou's avatar Ramon Nou
Browse files

fix(client): avoid null callback race during interception shutdown

parent 9ba40ae1

src/client/intercept.cpp

+16 −3
Changes for src/client/intercept.cpp: 16 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ 


#include <cerrno>

#include <cstring>

#include <atomic>



extern "C" {

#include <syscall.h>
@@ -80,6 +81,8 @@ void (*intercept_hook_point_post_kernel)(long syscall_number, long arg0, 
#endif

namespace {



std::atomic<bool> auxiliary_hooks_enabled{true};



static pthread_key_t reentrance_guard_key;

static pthread_once_t key_once_control = PTHREAD_ONCE_INIT;

static void
@@ -1310,6 +1313,8 @@ socketcall_wrapper(long syscall_number, long& arg0, long& arg1, long& arg2, 
void

hook_forwarded_syscall(long syscall_number, long arg0, long arg1, long arg2,

                       long arg3, long arg4, long arg5, long result) {

    if(!auxiliary_hooks_enabled.load(std::memory_order_acquire))

        return;



    pthread_once(&key_once_control, make_key);

    if(pthread_getspecific(reentrance_guard_key) != NULL) {
@@ -1343,6 +1348,8 @@ void 
hook_clone_at_child(unsigned long flags, void* child_stack, int* ptid,

                    // cppcheck-suppress constParameterCallback

                    int* ctid, long newtls) {

    if(!auxiliary_hooks_enabled.load(std::memory_order_acquire))

        return;



#if defined(GKFS_ENABLE_LOGGING) && defined(GKFS_DEBUG_BUILD)

    const long args[gkfs::syscall::MAX_ARGS] = {
@@ -1367,6 +1374,8 @@ void 
hook_clone_at_parent(unsigned long flags, void* child_stack, int* ptid,

                     // cppcheck-suppress constParameterCallback

                     int* ctid, long newtls, long returned_pid) {

    if(!auxiliary_hooks_enabled.load(std::memory_order_acquire))

        return;



#if defined(GKFS_ENABLE_LOGGING) && defined(GKFS_DEBUG_BUILD)

    const long args[gkfs::syscall::MAX_ARGS] = {
@@ -1528,6 +1537,7 @@ void 
start_self_interception() {



    LOG(DEBUG, "Enabling syscall interception for self");

    auxiliary_hooks_enabled.store(true, std::memory_order_release);



    intercept_hook_point = internal_hook_guard_wrapper;

    intercept_hook_point_post_kernel = hook_forwarded_syscall;
@@ -1541,6 +1551,7 @@ start_interception() { 
    assert(CTX->interception_enabled());



    LOG(DEBUG, "Enabling syscall interception for client process");

    auxiliary_hooks_enabled.store(true, std::memory_order_release);



    // Set up the callback function pointer

    intercept_hook_point = hook_guard_wrapper;
@@ -1557,9 +1568,11 @@ stop_interception() { 
    // syscalls (during exit/shutdown/atexit) are forwarded to kernel instead of

    // triggering SIGTRAP.

    intercept_hook_point = passthrough_hook_wrapper;

    intercept_hook_point_post_kernel = nullptr;

    intercept_hook_point_clone_child = nullptr;

    intercept_hook_point_clone_parent = nullptr;

    // syscall_intercept checks these pointers before loading them again for

    // the call. Clearing them races with runtime threads still in a syscall

    // (including during atexit), allowing a call through nullptr. Leave the

    // callbacks installed and make them no-ops through an atomic flag instead.

    auxiliary_hooks_enabled.store(false, std::memory_order_release);

}



} // namespace gkfs::preload

tests/integration/syscalls/test_preload_threaded_io.py

+49 −0
Changes for tests/integration/syscalls/test_preload_threaded_io.py: 49 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -38,3 +38,52 @@ PY 
        timeout=60,

    )

    assert command.exit_code == 0, command.stderr.decode(errors="replace")





def test_stop_interception_preserves_callbacks(gkfs_shell, gkfs_daemon):

    """Stopping interception must not null callbacks used by runtime threads."""

    command = gkfs_shell.run(sys.executable, "-c", """

import ctypes

import os

import threading



lib = ctypes.CDLL(None)

stop = getattr(lib, '_ZN4gkfs7preload17stop_interceptionEv')

start = getattr(lib, '_ZN4gkfs7preload18start_interceptionEv')

stop.restype = start.restype = None

names = ('intercept_hook_point_post_kernel',

         'intercept_hook_point_clone_child',

         'intercept_hook_point_clone_parent')

callbacks = [ctypes.c_void_p.in_dll(lib, name) for name in names]

original = [callback.value for callback in callbacks]

assert all(original)

stop()

try:

    assert [callback.value for callback in callbacks] == original

finally:

    start()



done = threading.Event()

ready = threading.Barrier(5)

def worker():

    ready.wait()

    while not done.is_set():

        os.stat('/tmp')



threads = [threading.Thread(target=worker) for _ in range(4)]

for thread in threads:

    thread.start()

ready.wait()

try:

    for _ in range(2000):

        stop()

        start()

finally:

    start()

    done.set()

    for thread in threads:

        thread.join()

print('PASS interception callback shutdown race')

""", timeout=60, env={"LIBGKFS_LOG": "none"})

    assert command.exit_code == 0, command.stderr.decode(errors="replace")

    assert b"PASS interception callback shutdown race" in command.stdout
 
 No newline at end of file