From bd43448af2ef80afd38b2396351815b04ed6b727 Mon Sep 17 00:00:00 2001 From: TapTap Date: Sun, 13 Sep 2026 10:50:53 +0200 Subject: [PATCH] fix(daemon): bound per-source table lifetime and recompute occupancy The per-source host table only grew: once its fixed open-addressed table filled, host_intern returned -1 and the per-host cap plus the shared auth lockout silently failed open forever. Add a bounded-lifetime eviction policy: track a per-bucket last-use time and, when no empty bucket exists, atomically repurpose the first bucket that has no active connection and either has an expired lockout or has been idle, resetting its counters. Warn (rate-limited) on the genuine fail-open path. A child SIGKILLed mid-registration could also leak a module/host count because the parent only decremented on a REGISTERED slot. Make the slot table the source of truth: after the SIGCHLD reap the parent recomputes module_active[]/host_active[] from the surviving REGISTERED slots (atomics only, async-signal-safe) so any leaked increment is erased. Also clamp module_count to DAEMON_LIMITS_MAX_MODULES and use one helper for the sizing/register host-tracking condition (a lockout threshold with duration 0 is a no-op and must not intern hosts). --- src/shared/daemon_limits.c | 183 +++++++++++++++++++++++++++++-------- src/shared/daemon_limits.h | 59 ++++++++++-- src/shared/transport_tcp.c | 48 ++++++++-- tests/test_daemon_limits.c | 78 ++++++++++++++++ 4 files changed, 315 insertions(+), 53 deletions(-) diff --git a/src/shared/daemon_limits.c b/src/shared/daemon_limits.c index 2ba7e4e..edb5819 100644 --- a/src/shared/daemon_limits.c +++ b/src/shared/daemon_limits.c @@ -1,4 +1,6 @@ #include "daemon_limits.h" +#include "daemon_conf.h" +#include "log.h" #include #include #include @@ -8,6 +10,12 @@ #include #include +/* The two module-count bounds must agree: the daemon config parser never + * produces more than DAEMON_CONF_MAX_MODULES modules, so the shared registry's + * per-module counter array is sized from the same bound. */ +_Static_assert(DAEMON_LIMITS_MAX_MODULES == DAEMON_CONF_MAX_MODULES, + "daemon_limits module bound must match daemon_conf"); + /* Slot lifecycle states (stored in slot_state). */ enum { SLOT_FREE = 0, @@ -27,6 +35,7 @@ struct DaemonLimitRegistry { int lockout_threshold; int lockout_duration_sec; size_t map_size; + _Atomic long long host_full_warn; /* last "table full" warning epoch */ _Atomic int* slot_state; _Atomic int* slot_pid; _Atomic int* slot_module; @@ -35,7 +44,8 @@ struct DaemonLimitRegistry { _Atomic uint64_t* host_key; /* 0 == empty bucket */ _Atomic int* host_active; _Atomic int* host_fail; - _Atomic long long* host_until; /* epoch seconds the lockout expires */ + _Atomic long long* host_until; /* epoch seconds the lockout expires */ + _Atomic long long* host_last_use; /* epoch seconds the bucket was last touched */ }; static size_t round_up(size_t n, size_t align) { @@ -88,7 +98,17 @@ uint64_t daemon_limits_host_hash(const char* peer_ip, bool* ok) { return hash; } -/* Find the bucket holding `peer_ip`, or -1 when it has no entry. */ +/* True when the registry must maintain per-source buckets: either the per-host + * cap is configured, or the auth lockout is (threshold AND duration > 0). A + * lockout threshold without a duration is a no-op, so it must not size or intern + * the table. create(), register() and the lockout paths all agree on this. */ +static bool registry_tracks_hosts(const DaemonLimitRegistry* registry) { + return registry->per_host_cap > 0 || + (registry->lockout_threshold > 0 && registry->lockout_duration_sec > 0); +} + +/* Find the bucket holding `peer_ip`, or -1 when it has no entry. Finding a + * bucket refreshes its last-use time so the eviction policy sees it as live. */ static int host_lookup(DaemonLimitRegistry* registry, const char* peer_ip) { bool ok = false; uint64_t key = daemon_limits_host_hash(peer_ip, &ok); @@ -99,39 +119,112 @@ static int host_lookup(DaemonLimitRegistry* registry, const char* peer_ip) { for (size_t i = 0; i < (size_t)registry->host_slots; i++) { size_t idx = (start + i) & mask; uint64_t current = atomic_load_explicit(®istry->host_key[idx], memory_order_acquire); - if (current == key) + if (current == key) { + atomic_store_explicit(®istry->host_last_use[idx], (long long)time(NULL), + memory_order_relaxed); return (int)idx; + } if (current == 0) return -1; /* no tombstones: an empty bucket ends the probe chain */ } return -1; } +/* A bucket with no live connection may be repurposed: immediately when its + * lockout deadline has already passed (the review's "expired" case), or after an + * idle window when it holds no pending lockout. A bucket with a future lockout + * deadline is retained so the lockout actually lasts its configured duration. */ +static bool host_bucket_reclaimable(DaemonLimitRegistry* registry, size_t idx, long long now) { + if (atomic_load_explicit(®istry->host_active[idx], memory_order_relaxed) != 0) + return false; + long long until = atomic_load_explicit(®istry->host_until[idx], memory_order_relaxed); + if (until != 0) + return until <= now; + long long last_use = atomic_load_explicit(®istry->host_last_use[idx], memory_order_relaxed); + return last_use == 0 || now - last_use >= DAEMON_LIMITS_HOST_EVICT_IDLE_SEC; +} + +/* Emit at most one "per-source table full" warning per + * DAEMON_LIMITS_HOST_FULL_WARN_SEC across all forked children. Called from a + * normal (non-signal) child path, so logging is safe here. */ +static void host_warn_table_full(DaemonLimitRegistry* registry, long long now) { + long long last = atomic_load_explicit(®istry->host_full_warn, memory_order_relaxed); + if (last != 0 && now - last < DAEMON_LIMITS_HOST_FULL_WARN_SEC) + return; + if (atomic_compare_exchange_strong_explicit(®istry->host_full_warn, &last, now, + memory_order_relaxed, memory_order_relaxed)) { + log_message(LOG_LEVEL_WARNING, + "daemon: per-source registry is full (%d slots) and no bucket can be reclaimed; " + "'max connections per host' and the auth lockout are temporarily not enforced for " + "new sources (the per-module cap and host ACLs still apply)", + registry->host_slots); + } +} + /* Find or insert the bucket for `peer_ip`. Insertion is a lock-free CAS so two - * forked children racing on the same source converge on one bucket. Returns -1 - * when the table is full or the address is unparseable (callers fail open: the - * global/module caps and ACLs still apply). */ + * forked children racing on the same source converge on one bucket. + * + * When the probe finds no empty bucket it reclaims, via a key CAS, the first + * bucket that is reclaimable (expired lockout or idle, and no active + * connection) and resets its counters. This bounds the table's lifetime so it + * cannot fill permanently and stay fail-open. Returns -1 only when the address + * is unparseable or the table is genuinely full of live/locked buckets + * (callers fail open: the global/module caps and ACLs still apply). */ static int host_intern(DaemonLimitRegistry* registry, const char* peer_ip) { bool ok = false; uint64_t key = daemon_limits_host_hash(peer_ip, &ok); if (!ok) return -1; + long long now = (long long)time(NULL); size_t mask = (size_t)registry->host_slots - 1; size_t start = (size_t)(key & mask); - for (size_t i = 0; i < (size_t)registry->host_slots; i++) { - size_t idx = (start + i) & mask; - uint64_t current = atomic_load_explicit(®istry->host_key[idx], memory_order_acquire); - if (current == key) - return (int)idx; - if (current == 0) { - uint64_t expected = 0; - if (atomic_compare_exchange_strong_explicit(®istry->host_key[idx], &expected, key, - memory_order_acq_rel, memory_order_acquire)) - return (int)idx; - if (atomic_load_explicit(®istry->host_key[idx], memory_order_acquire) == key) + /* A couple of passes bound the work: the first normally claims/seeds a bucket; + * a lost eviction CAS retries once against the freshly observed table. */ + for (int pass = 0; pass < 2; pass++) { + int evict = -1; + uint64_t evict_key = 0; + for (size_t i = 0; i < (size_t)registry->host_slots; i++) { + size_t idx = (start + i) & mask; + uint64_t current = atomic_load_explicit(®istry->host_key[idx], memory_order_acquire); + if (current == key) { + atomic_store_explicit(®istry->host_last_use[idx], now, memory_order_relaxed); return (int)idx; + } + if (current == 0) { + uint64_t expected = 0; + if (atomic_compare_exchange_strong_explicit(®istry->host_key[idx], &expected, key, + memory_order_acq_rel, memory_order_acquire)) { + atomic_store_explicit(®istry->host_last_use[idx], now, memory_order_relaxed); + return (int)idx; + } + if (atomic_load_explicit(®istry->host_key[idx], memory_order_acquire) == key) { + atomic_store_explicit(®istry->host_last_use[idx], now, memory_order_relaxed); + return (int)idx; + } + continue; /* another child won this empty bucket; keep probing */ + } + if (evict < 0 && host_bucket_reclaimable(registry, idx, now)) { + evict = (int)idx; + evict_key = current; + } } + if (evict >= 0) { + uint64_t expected = evict_key; + if (atomic_compare_exchange_strong_explicit(®istry->host_key[evict], &expected, key, + memory_order_acq_rel, memory_order_acquire)) { + /* The bucket now belongs to the new source; clear the evicted source's + * stale lockout/failure state. */ + atomic_store_explicit(®istry->host_active[evict], 0, memory_order_relaxed); + atomic_store_explicit(®istry->host_fail[evict], 0, memory_order_relaxed); + atomic_store_explicit(®istry->host_until[evict], 0, memory_order_relaxed); + atomic_store_explicit(®istry->host_last_use[evict], now, memory_order_relaxed); + return evict; + } + continue; /* lost the race; re-probe with fresh observations */ + } + break; /* no free and no reclaimable bucket: genuinely full */ } + host_warn_table_full(registry, now); return -1; } @@ -143,6 +236,8 @@ DaemonLimitRegistry* daemon_limits_create(int max_slots, int module_count, int p max_slots = DAEMON_LIMITS_MAX_SLOTS; if (module_count < 1) module_count = 1; + if (module_count > DAEMON_LIMITS_MAX_MODULES) + module_count = DAEMON_LIMITS_MAX_MODULES; if (per_host_cap < 0) per_host_cap = 0; if (lockout_threshold < 0) @@ -167,7 +262,7 @@ DaemonLimitRegistry* daemon_limits_create(int max_slots, int module_count, int p size_t module_bytes = round_up((size_t)module_count * sizeof(_Atomic int), 16); size_t host_key_bytes = round_up((size_t)host_slots * sizeof(_Atomic uint64_t), 16); size_t host_int_bytes = round_up((size_t)host_slots * sizeof(_Atomic int), 16) * 2; - size_t host_until_bytes = round_up((size_t)host_slots * sizeof(_Atomic long long), 16); + size_t host_until_bytes = round_up((size_t)host_slots * sizeof(_Atomic long long), 16) * 2; size_t total = header + slot_bytes + module_bytes + host_key_bytes + host_int_bytes + host_until_bytes + 16; @@ -205,6 +300,8 @@ DaemonLimitRegistry* daemon_limits_create(int max_slots, int module_count, int p cursor += (size_t)host_slots * sizeof(_Atomic int); cursor = (unsigned char*)round_up((size_t)(uintptr_t)cursor, 16); registry->host_until = (atomic_llong*)cursor; + cursor += (size_t)host_slots * sizeof(_Atomic long long); + registry->host_last_use = (atomic_llong*)cursor; for (int i = 0; i < max_slots; i++) { atomic_store(®istry->slot_module[i], -1); @@ -243,25 +340,12 @@ void daemon_limits_set_slot_pid(DaemonLimitRegistry* registry, int slot, long pi void daemon_limits_reclaim_slot(DaemonLimitRegistry* registry, int slot) { if (!registry || slot < 0 || slot >= registry->max_slots) return; - int previous = - atomic_exchange_explicit(®istry->slot_state[slot], SLOT_FREE, memory_order_acq_rel); - if (previous == SLOT_REGISTERED) { - int module = atomic_load(®istry->slot_module[slot]); - int host = atomic_load(®istry->slot_host[slot]); - if (module >= 0 && module < registry->module_count) { - int current = atomic_load(®istry->module_active[module]); - while (current > 0 && - !atomic_compare_exchange_weak(®istry->module_active[module], ¤t, current - 1)) - ; - } - if (host >= 0 && host < registry->host_slots) { - int current = atomic_load(®istry->host_active[host]); - while (current > 0 && - !atomic_compare_exchange_weak(®istry->host_active[host], ¤t, current - 1)) - ; - } - } - atomic_store(®istry->slot_pid[slot], 0); + atomic_exchange_explicit(®istry->slot_state[slot], SLOT_FREE, memory_order_acq_rel); + atomic_store_explicit(®istry->slot_pid[slot], 0, memory_order_relaxed); + /* The module/host occupancy arrays are derived from the slot table; do not + * decrement here or a SIGKILL between a child's increment and its REGISTERED + * publish would leak a count. Callers that need the derived counts call + * daemon_limits_recompute. */ } void daemon_limits_reclaim_pid(DaemonLimitRegistry* registry, long pid) { @@ -277,6 +361,29 @@ void daemon_limits_reclaim_pid(DaemonLimitRegistry* registry, long pid) { } } +void daemon_limits_recompute(DaemonLimitRegistry* registry) { + if (!registry) + return; + /* Zero the derived arrays, then re-derive solely from the REGISTERED slots. + * A child that was SIGKILLed after incrementing a counter but before + * publishing REGISTERED is not counted, and its leaked increment is erased by + * the zeroing, so the leak cannot persist. */ + for (int m = 0; m < registry->module_count; m++) + atomic_store_explicit(®istry->module_active[m], 0, memory_order_relaxed); + for (int h = 0; h < registry->host_slots; h++) + atomic_store_explicit(®istry->host_active[h], 0, memory_order_relaxed); + for (int i = 0; i < registry->max_slots; i++) { + if (atomic_load_explicit(®istry->slot_state[i], memory_order_acquire) != SLOT_REGISTERED) + continue; + int module = atomic_load_explicit(®istry->slot_module[i], memory_order_relaxed); + if (module >= 0 && module < registry->module_count) + atomic_fetch_add_explicit(®istry->module_active[module], 1, memory_order_relaxed); + int host = atomic_load_explicit(®istry->slot_host[i], memory_order_relaxed); + if (host >= 0 && host < registry->host_slots) + atomic_fetch_add_explicit(®istry->host_active[host], 1, memory_order_relaxed); + } +} + DaemonLimitResult daemon_limits_register(DaemonLimitRegistry* registry, int slot, int module_index, const char* peer_ip, int module_cap) { if (!registry || slot < 0 || slot >= registry->max_slots) @@ -287,7 +394,7 @@ DaemonLimitResult daemon_limits_register(DaemonLimitRegistry* registry, int slot return DAEMON_LIMIT_UNAVAILABLE; int host = -1; - if (registry->per_host_cap > 0 || registry->lockout_threshold > 0) + if (registry_tracks_hosts(registry)) host = host_intern(registry, peer_ip); int module_count = atomic_fetch_add(®istry->module_active[module_index], 1) + 1; diff --git a/src/shared/daemon_limits.h b/src/shared/daemon_limits.h index 47bfb0b..b6d89d2 100644 --- a/src/shared/daemon_limits.h +++ b/src/shared/daemon_limits.h @@ -34,6 +34,20 @@ * already collapsed to IPv4 by utils_fd_peer_ip); it is interned into an * open-addressed, linear-probing table keyed by a 64-bit hash. The same table * also carries the cross-process auth-failure counter and lockout deadline. + * + * Per-source table lifetime: a bucket's key is never cleared back to empty (that + * would break every later probe chain that passed through it). Instead the + * table has a bounded-lifetime eviction policy: when no empty bucket exists, the + * first bucket that is reclaimable -- no active connection AND (its lockout + * deadline has passed OR it has been idle for + * DAEMON_LIMITS_HOST_EVICT_IDLE_SEC) -- is atomically repurposed for the new + * source via a CAS of its key, and its counters are reset. The table therefore + * cannot fill permanently, and a full table degrades to fail-open for the + * per-source cap/lockout of new sources (the per-module cap and host ACLs still + * apply) instead of staying fail-open forever. A rate-limited warning is logged + * on the fail-open path. The eviction race with a concurrent + * registration/reclaim on the same bucket is benign: it can at worst lose one + * source's counter (fail-open), never corrupt memory or the module caps. */ typedef struct DaemonLimitRegistry DaemonLimitRegistry; @@ -51,13 +65,27 @@ typedef enum { #define DAEMON_LIMITS_MAX_SLOTS 65536 #define DAEMON_LIMITS_MAX_HOST_SLOTS 65536 #define DAEMON_LIMITS_NO_SLOT (-1) +/* Upper bound on `module_count`, matching daemon_conf.h's DAEMON_CONF_MAX_MODULES + * (asserted in daemon_limits.c) so a caller can never size the per-module counter + * array larger than the config parser can produce. */ +#define DAEMON_LIMITS_MAX_MODULES 256 + +/* Per-source table lifetime: a bucket with no active connection and no pending + * lockout is reclaimable once it has been idle this long, so a flood of distinct + * sources cannot pin the table full forever. A bucket whose lockout deadline + * has passed is reclaimable immediately (independent of this idle window). */ +#define DAEMON_LIMITS_HOST_EVICT_IDLE_SEC 300 +/* Minimum spacing between "per-source table is full" warnings, so a table-full + * attack cannot flood the log. */ +#define DAEMON_LIMITS_HOST_FULL_WARN_SEC 60 /* Create the shared registry in the calling (parent) process. `max_slots` is * the number of concurrently live children to track (clamped to * [DAEMON_LIMITS_MIN_SLOTS, DAEMON_LIMITS_MAX_SLOTS]); `module_count` is the - * number of daemon modules (clamped to >= 1); `per_host_cap` and the lockout - * pair come from the daemon config (0 disables). Returns NULL on failure (e.g. - * mmap allocation); callers must degrade gracefully (global cap + ACLs still + * number of daemon modules (clamped to + * [1, DAEMON_LIMITS_MAX_MODULES]); `per_host_cap` and the lockout pair come + * from the daemon config (0 disables). Returns NULL on failure (e.g. mmap + * allocation); callers must degrade gracefully (global cap + ACLs still * apply). */ DaemonLimitRegistry* daemon_limits_create(int max_slots, int module_count, int per_host_cap, int lockout_threshold, int lockout_duration_sec); @@ -70,16 +98,33 @@ void daemon_limits_destroy(DaemonLimitRegistry* registry); int daemon_limits_claim_slot(DaemonLimitRegistry* registry); /* Parent side: record the forked child's pid in a claimed slot. */ void daemon_limits_set_slot_pid(DaemonLimitRegistry* registry, int slot, long pid); -/* Parent side: release a slot, decrementing the module/per-source counters when - * the slot was actually REGISTERED. Idempotent. */ +/* Parent side: release a slot. The slot becomes FREE; the module/per-source + * occupancy arrays are DERIVED state and are only refreshed by + * daemon_limits_recompute, which callers must invoke afterwards when they rely + * on the derived counts (the SIGCHLD handler batches one recompute for the whole + * reap). Idempotent. */ void daemon_limits_reclaim_slot(DaemonLimitRegistry* registry, int slot); -/* Parent SIGCHLD side: reclaim the slot owned by `pid` (no-op when not found). */ +/* Parent SIGCHLD side: release the slot owned by `pid` (no-op when not found). + * Like reclaim_slot this does not touch the derived occupancy arrays; call + * daemon_limits_recompute after a batch of releases. */ void daemon_limits_reclaim_pid(DaemonLimitRegistry* registry, long pid); +/* Parent side (async-signal-safe; atomics only, no malloc/log): rebuild + * module_active[] / host_active[] from scratch by scanning the REGISTERED slots. + * The slot table is the single source of truth, so this self-heals any + * count leaked by a child that was SIGKILLed mid-registration (it zeroes the + * arrays and re-derives them). Bounded by max_slots + host_slots. A + * registration racing this call can be transiently undercounted until the next + * recompute, which can only relax a cap briefly -- never corrupt memory. */ +void daemon_limits_recompute(DaemonLimitRegistry* registry); + /* Child side: admit the connection for `module_index` from `peer_ip`. Always * tracks the module/per-source occupancy (so the parent's reclaim is * symmetric); when `module_cap` > 0 it additionally enforces the per-module - * cap. Returns DAEMON_LIMIT_OK and publishes the slot, or a refusal reason. */ + * cap. A NULL/empty or non-numeric `peer_ip` skips the per-source track (the + * callers use that to exempt a trusted loopback peer from the per-host cap; the + * per-module cap still applies). Returns DAEMON_LIMIT_OK and publishes the + * slot, or a refusal reason. */ DaemonLimitResult daemon_limits_register(DaemonLimitRegistry* registry, int slot, int module_index, const char* peer_ip, int module_cap); diff --git a/src/shared/transport_tcp.c b/src/shared/transport_tcp.c index e73dbce..9fedfb7 100644 --- a/src/shared/transport_tcp.c +++ b/src/shared/transport_tcp.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -39,9 +40,25 @@ static void sigchld_handler(int sig) { g_active_connections--; daemon_limits_reclaim_pid(g_limit_registry, (long)pid); } + /* Re-derive the occupancy counters once for the whole reap batch. The slot + * table is the source of truth, so this self-heals any count leaked by a child + * SIGKILLed mid-registration. Atomics only: async-signal-safe. */ + if (g_limit_registry) + daemon_limits_recompute(g_limit_registry); errno = saved_errno; } +/* Reset a signal to its default action with sigaction (preferred over + * signal(3), whose semantics are implementation-defined). Used in the forked + * child before it can spawn any thread. */ +static void reset_signal_default(int sig) { + struct sigaction action; + memset(&action, 0, sizeof(action)); + action.sa_handler = SIG_DFL; + sigemptyset(&action.sa_mask); + sigaction(sig, &action, NULL); +} + /* Map a listen socket's address to its numeric port for logging, independent * of whether it is an IPv4 or IPv6 sockaddr. */ static unsigned short server_address_port(const struct sockaddr_storage* addr) { @@ -160,7 +177,16 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil log_perror("Could not listen on port!"); return; } - signal(SIGCHLD, sigchld_handler); + /* SIGCHLD via sigaction (not signal(3)); SA_RESTART keeps accept(2) from + * failing with EINTR, and SA_NOCLDSTOP only notifies on child exit. The + * accept loop is single-threaded at this point, so installing here cannot race + * a worker thread. */ + struct sigaction chld_action; + memset(&chld_action, 0, sizeof(chld_action)); + chld_action.sa_handler = sigchld_handler; + sigemptyset(&chld_action.sa_mask); + chld_action.sa_flags = SA_RESTART | SA_NOCLDSTOP; + sigaction(SIGCHLD, &chld_action, NULL); g_limit_registry = server->limit_registry; while (1) { struct sockaddr_storage client_addr; @@ -197,15 +223,21 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil g_current_slot = slot; /* Block SIGCHLD across fork() and the parent's pid publication: a child * that exits immediately must not be reaped before its slot records its - * pid, which would leak the slot and its module/source counts. */ + * pid, which would leak the slot and its module/source counts. Use + * pthread_sigmask rather than sigprocmask so the behavior is well defined + * even if this process ever gains threads: the mask is per-thread, the fork + * copies only the calling thread, and the child inherits this thread's + * blocked mask until it restores `previous` below. No thread exists yet at + * this point, and none is created before the mask is restored, so the + * critical window is race-free. */ sigset_t blocked; sigset_t previous; sigemptyset(&blocked); sigaddset(&blocked, SIGCHLD); - sigprocmask(SIG_BLOCK, &blocked, &previous); + pthread_sigmask(SIG_BLOCK, &blocked, &previous); pid_t pid = fork(); if (pid == 0) { - sigprocmask(SIG_SETMASK, &previous, NULL); + pthread_sigmask(SIG_SETMASK, &previous, NULL); /* Connection children must not run the parent's global cleanup(): it * frees state (credentials / daemon conf) that the child's worker * threads may still be reading and closes fd numbers the child could @@ -213,9 +245,9 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil * terminates the child directly; SIGCHLD is reset too since a child * must never reap the parent's children. This runs before the child * spawns any thread, so it cannot race one. */ - signal(SIGINT, SIG_DFL); - signal(SIGTERM, SIG_DFL); - signal(SIGCHLD, SIG_DFL); + reset_signal_default(SIGINT); + reset_signal_default(SIGTERM); + reset_signal_default(SIGCHLD); close(server->file_descriptor); child_fn(fd, child_ctx); _exit(0); @@ -227,7 +259,7 @@ static void accept_loop(Server* server, void (*child_fn)(int, void*), void* chil /* fork() failed: release the reservation so the slot is not leaked. */ daemon_limits_reclaim_slot(server->limit_registry, slot); } - sigprocmask(SIG_SETMASK, &previous, NULL); + pthread_sigmask(SIG_SETMASK, &previous, NULL); close(fd); } } diff --git a/tests/test_daemon_limits.c b/tests/test_daemon_limits.c index 2815e0a..8f36bb0 100644 --- a/tests/test_daemon_limits.c +++ b/tests/test_daemon_limits.c @@ -2,7 +2,9 @@ #include "daemon_limits.h" #include "test_utils.h" #include +#include #include +#include #include /* The per-source hash is a pure helper: numeric addresses hash to a nonzero, @@ -72,6 +74,7 @@ static void test_daemon_limits_module_cap() { daemon_limits_reclaim_slot(registry, slot0); daemon_limits_reclaim_slot(registry, slot1); + daemon_limits_recompute(registry); int slot4 = daemon_limits_claim_slot(registry); EXPECT_TRUE(slot4 >= 0); EXPECT_EQ_INT(daemon_limits_register(registry, slot4, 0, "10.0.0.4", 2), DAEMON_LIMIT_OK); @@ -94,6 +97,7 @@ static void test_daemon_limits_host_cap() { EXPECT_EQ_INT(daemon_limits_register(registry, slot2, 0, "10.0.0.2", 0), DAEMON_LIMIT_OK); /* Reclaiming the first source frees its per-host allowance. */ daemon_limits_reclaim_slot(registry, slot0); + daemon_limits_recompute(registry); EXPECT_EQ_INT(daemon_limits_register(registry, slot1, 0, "10.0.0.1", 0), DAEMON_LIMIT_OK); daemon_limits_destroy(registry); @@ -115,6 +119,7 @@ static void test_daemon_limits_reclaim_pid() { DAEMON_LIMIT_MODULE_FULL); daemon_limits_reclaim_pid(registry, 4242); + daemon_limits_recompute(registry); EXPECT_EQ_INT(daemon_limits_register(registry, slot1, 0, "10.0.0.1", 1), DAEMON_LIMIT_OK); /* Reclaiming an unknown pid is a no-op. */ daemon_limits_reclaim_pid(registry, 999999); @@ -179,16 +184,89 @@ static void test_daemon_limits_fork_shared() { DAEMON_LIMIT_MODULE_FULL); /* The parent reclaims the dead child's slot by pid. */ daemon_limits_reclaim_pid(registry, (long)pid); + daemon_limits_recompute(registry); EXPECT_EQ_INT(daemon_limits_register(registry, slot1, 0, "10.0.0.2", 1), DAEMON_LIMIT_OK); daemon_limits_destroy(registry); } +/* The occupancy arrays are derived from the slot table: recompute rebuilds them + * and is the self-heal path the SIGCHLD handler uses after a child dies. */ +static void test_daemon_limits_recompute() { + DaemonLimitRegistry* registry = daemon_limits_create(DAEMON_LIMITS_MIN_SLOTS, 2, 1, 0, 0); + EXPECT_NOT_NULL(registry); + int slot0 = daemon_limits_claim_slot(registry); + int slot1 = daemon_limits_claim_slot(registry); + int slot2 = daemon_limits_claim_slot(registry); + EXPECT_TRUE(slot0 >= 0 && slot1 >= 0 && slot2 >= 0); + EXPECT_EQ_INT(daemon_limits_register(registry, slot0, 0, "10.0.0.1", 0), DAEMON_LIMIT_OK); + EXPECT_EQ_INT(daemon_limits_register(registry, slot1, 0, "10.0.0.2", 0), DAEMON_LIMIT_OK); + + /* Recompute is idempotent and re-derives the same counts from REGISTERED + * slots (a CLAIMED slot is never counted). */ + daemon_limits_recompute(registry); + daemon_limits_recompute(registry); + EXPECT_EQ_INT(daemon_limits_register(registry, slot2, 0, "10.0.0.3", 2), + DAEMON_LIMIT_MODULE_FULL); + + /* Freeing a slot and recomputing releases its module/per-source count. */ + daemon_limits_reclaim_slot(registry, slot0); + daemon_limits_recompute(registry); + EXPECT_EQ_INT(daemon_limits_register(registry, slot2, 0, "10.0.0.3", 2), DAEMON_LIMIT_OK); + daemon_limits_destroy(registry); +} + +/* The per-source table has a bounded lifetime. When every bucket is occupied + * but not yet reclaimable, a new source is fail-open: the per-host cap is not + * enforced and the probe must terminate. Once the occupied buckets' lockouts + * expire (or they go idle), a new source reclaims a bucket and enforcement comes + * back. This covers the "table never evicts -> cap silently fails open forever" + * review finding. */ +static void test_daemon_limits_host_table_eviction() { + char ip[32]; + + /* Part A: all buckets locked out with a long deadline and no active + * connection are not reclaimable yet. A new source cannot be interned, so the + * per-host cap is documented fail-open (both connections admitted) -- and the + * bounded probe returns instead of looping forever. */ + DaemonLimitRegistry* registry = daemon_limits_create(DAEMON_LIMITS_MIN_SLOTS, 1, 1, 1, 300); + EXPECT_NOT_NULL(registry); + for (int i = 0; i < 64; i++) { + snprintf(ip, sizeof(ip), "10.0.0.%d", i + 1); + daemon_limits_auth_record_failure(registry, ip); + } + int a = daemon_limits_claim_slot(registry); + int b = daemon_limits_claim_slot(registry); + EXPECT_TRUE(a >= 0 && b >= 0); + EXPECT_EQ_INT(daemon_limits_register(registry, a, 0, "10.9.9.9", 0), DAEMON_LIMIT_OK); + EXPECT_EQ_INT(daemon_limits_register(registry, b, 0, "10.9.9.9", 0), DAEMON_LIMIT_OK); + daemon_limits_destroy(registry); + + /* Part B: with an already-expired lockout every bucket is reclaimable, so a + * new source reclaims one and the per-host cap is enforced again. */ + registry = daemon_limits_create(DAEMON_LIMITS_MIN_SLOTS, 1, 1, 1, 1); + EXPECT_NOT_NULL(registry); + for (int i = 0; i < 64; i++) { + snprintf(ip, sizeof(ip), "10.0.0.%d", i + 1); + daemon_limits_auth_record_failure(registry, ip); + } + struct timespec pause = {2, 0}; + nanosleep(&pause, NULL); + int c = daemon_limits_claim_slot(registry); + int d = daemon_limits_claim_slot(registry); + EXPECT_TRUE(c >= 0 && d >= 0); + EXPECT_EQ_INT(daemon_limits_register(registry, c, 0, "10.9.9.9", 0), DAEMON_LIMIT_OK); + EXPECT_EQ_INT(daemon_limits_register(registry, d, 0, "10.9.9.9", 0), DAEMON_LIMIT_HOST_FULL); + daemon_limits_destroy(registry); +} + void test_daemon_limits() { test_daemon_limits_host_hash(); test_daemon_limits_slots(); test_daemon_limits_module_cap(); test_daemon_limits_host_cap(); test_daemon_limits_reclaim_pid(); + test_daemon_limits_recompute(); test_daemon_limits_auth_lockout(); + test_daemon_limits_host_table_eviction(); test_daemon_limits_fork_shared(); }