fix(audit): security, correctness, refactors, docs (no wire change) #306

Merged
TapTap merged 44 commits from fix/audit-cycle into dev 2026-09-21 22:23:33 +02:00
2 changed files with 49 additions and 11 deletions
Showing only changes of commit df8fe1ae4e - Show all commits
+16 -3
View File
@@ -54,13 +54,26 @@ bool client_abort_pending(void) {
} }
#ifndef FASTSYNC_TEST_BUILD #ifndef FASTSYNC_TEST_BUILD
/* SIG_DFL disposition used by the handler's "not armed" fallback. It is built
* once at load time so the handler can restore the default action with
* sigaction(2) -- which is async-signal-safe -- instead of signal(3), which is
* not. The zero-initialized sa_mask is the empty set. */
static const struct sigaction client_default_action = {
.sa_handler = SIG_DFL,
.sa_flags = 0,
};
/* Signal handler: perform NO work beyond storing the flag. Logging, protocol /* Signal handler: perform NO work beyond storing the flag. Logging, protocol
* I/O and the STATUS_ABORT frame are all done later on the normal send path, * I/O and the STATUS_ABORT frame are all done later on the normal send path,
* which is not async-signal-safe. When no transfer is armed, fall back to the * which is not async-signal-safe. When no transfer is armed, restore the
* default action so local-only modes remain interruptible. */ * default disposition (async-signal-safe sigaction) and re-raise so local-only
* modes remain interruptible. The handler deliberately stays installed while a
* transfer is armed -- rather than using SA_RESETHAND -- so a second Ctrl-C
* during the graceful abort keeps setting the flag instead of hard-killing the
* process mid-cleanup. */
static void client_signal_handler(int signo) { static void client_signal_handler(int signo) {
if (!client_abort_armed) { if (!client_abort_armed) {
signal(signo, SIG_DFL); sigaction(signo, &client_default_action, NULL);
raise(signo); raise(signo);
return; return;
} }
+33 -8
View File
@@ -1055,17 +1055,42 @@ done:
#ifndef FASTSYNC_SERVER_AS_LIB #ifndef FASTSYNC_SERVER_AS_LIB
static Server* g_server = NULL; static Server* g_server = NULL;
/* Signal handler for the foreground daemon/standalone listener.
*
* Async-signal-safety: _exit(2) is on the POSIX async-signal-safe list and is
* the ONLY thing done here. The previous body called server_delete()
* (close/free/SSL_CTX_free), daemon_conf_free() and credentials_free(); none of
* those (free/malloc, and much of OpenSSL teardown) are async-signal-safe, so a
* signal delivered while the main thread was inside malloc/free could deadlock
* or corrupt the heap.
*
* Residual (documented, not hidden): the in-memory teardown is skipped on the
* signal path. That is safe because the parent daemon owns no persistent
* resource that survives process exit -- the listening socket is closed by the
* kernel, the connection registry is an anonymous MAP_SHARED mapping with no
* named backing object, and the daemon config/credential stores are plain heap
* allocations. Connection children are separate processes and handle their own
* temp files/locks. The normal (non-signal) shutdown path in main() still runs
* the full teardown, so no cleanup is dropped on the common path. Wiring the
* accept loop (transport_tcp.c, outside this change's scope) to a flag-based
* self-pipe shutdown would let the frees run context-safely; it is deliberately
* deferred rather than risk restructuring the daemon loop. */
static void cleanup(int sig) { static void cleanup(int sig) {
(void)sig; (void)sig;
if (g_server)
server_delete(&g_server);
daemon_conf_free(g_daemon_conf);
g_daemon_conf = NULL;
credentials_free(g_credentials);
g_credentials = NULL;
_exit(0); _exit(0);
} }
/* Install a signal handler with sigaction(2) (the required async-signal-safe
* install primitive; signal(3) is not specified to be async-signal-safe). */
static void install_cleanup_handler(int signo) {
struct sigaction action;
memset(&action, 0, sizeof(action));
action.sa_handler = cleanup;
sigemptyset(&action.sa_mask);
action.sa_flags = 0;
sigaction(signo, &action, NULL);
}
static void print_server_usage(void) { static void print_server_usage(void) {
printf("FastSync Server\n"); printf("FastSync Server\n");
printf("Usage: fastsync-server [options]\n\n"); printf("Usage: fastsync-server [options]\n\n");
@@ -1259,8 +1284,8 @@ int main(int argc, char* argv[]) {
* this process-global policy cannot be re-enabled by a future caller. */ * this process-global policy cannot be re-enabled by a future caller. */
server_allow_super = opts.allow_super && !opts.stdio_mode; server_allow_super = opts.allow_super && !opts.stdio_mode;
server_iconv_spec = opts.iconv_spec; server_iconv_spec = opts.iconv_spec;
signal(SIGINT, cleanup); install_cleanup_handler(SIGINT);
signal(SIGTERM, cleanup); install_cleanup_handler(SIGTERM);
/* Server-owned socket deadline floor: the client default --timeout=0 would /* Server-owned socket deadline floor: the client default --timeout=0 would
* otherwise leave accepted sockets without SO_RCVTIMEO/SO_SNDTIMEO and let a * otherwise leave accepted sockets without SO_RCVTIMEO/SO_SNDTIMEO and let a
* silent peer hold a connection (and its process slot) forever. */ * silent peer hold a connection (and its process slot) forever. */