From e04ccc67d7e83d13c33f4d001c6fdbaa2e1d274a Mon Sep 17 00:00:00 2001 From: "J. Kirby Ross" Date: Tue, 24 Jun 2025 09:01:07 -0700 Subject: [PATCH] fix: thread-unsafe global state in id.c using pthread_once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace race-prone boolean flag with pthread_once for SipHash key initialization. This ensures the key is initialized exactly once even with concurrent access. - Replace g_siphash_key_initialized flag with pthread_once_t - Use pthread_once() to guarantee single initialization - Add pthread to LDFLAGS in Makefile - Free error on fallback path to prevent memory leak Fixes TODO #1 from TASKLIST.md 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- core/Makefile | 4 ++-- core/src/types/id.c | 40 ++++++++++++++++------------------------ 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/core/Makefile b/core/Makefile index 9acf84c5..aee585eb 100644 --- a/core/Makefile +++ b/core/Makefile @@ -5,7 +5,7 @@ CC ?= gcc CFLAGS = -Wall -Wextra -Werror -std=gnu11 -O2 CFLAGS += -I./include -LDFLAGS = -lsodium +LDFLAGS = -lsodium -lpthread # Coverage settings COVERAGE ?= 0 @@ -75,7 +75,7 @@ $(OBJ_DIR)/%.o: $(SRC_DIR)/%.c test: dirs $(ALL_OBJS) $(CC) $(CFLAGS) $(TEST_ERROR) $(ERROR_OBJS) -o $(BIN_DIR)/test_error $(LDFLAGS) $(CC) $(CFLAGS) $(TEST_ID) $(TYPES_OBJS) $(CRYPTO_OBJS) $(UTF8_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_id $(LDFLAGS) - $(CC) $(CFLAGS) $(TEST_ID_THREAD_SAFETY) $(TYPES_OBJS) $(CRYPTO_OBJS) $(UTF8_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_id_thread_safety $(LDFLAGS) -lpthread + $(CC) $(CFLAGS) $(TEST_ID_THREAD_SAFETY) $(TYPES_OBJS) $(CRYPTO_OBJS) $(UTF8_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_id_thread_safety $(LDFLAGS) $(CC) $(CFLAGS) $(TEST_STRING) $(TYPES_OBJS) $(CRYPTO_OBJS) $(UTF8_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_string $(LDFLAGS) $(CC) $(CFLAGS) $(TEST_PATH) $(TYPES_OBJS) $(CRYPTO_OBJS) $(UTF8_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_path $(LDFLAGS) $(CC) $(CFLAGS) $(TEST_SHA256) $(CRYPTO_OBJS) $(ERROR_OBJS) -o $(BIN_DIR)/test_sha256 $(LDFLAGS) diff --git a/core/src/types/id.c b/core/src/types/id.c index 53f90f47..ed33778d 100644 --- a/core/src/types/id.c +++ b/core/src/types/id.c @@ -8,6 +8,7 @@ #include "gitmind/error.h" #include "gitmind/security/memory.h" +#include #include #include #include @@ -25,35 +26,26 @@ int gm_id_compare(gm_id_t id_a, gm_id_t id_b) { /* SipHash key - initialized once at startup */ static uint8_t g_siphash_key[crypto_shorthash_siphash24_KEYBYTES]; -static bool g_siphash_key_initialized = false; - -/* Initialize SipHash key with random data */ -static void ensure_siphash_key_initialized(void) { - if (!g_siphash_key_initialized) { - /* Generate random key - this is best effort, if it fails we use zeros - */ - gm_result_void result = - gm_random_bytes(g_siphash_key, sizeof(g_siphash_key)); - if (GM_IS_ERR(result)) { - /* If random generation fails, use a deterministic fallback */ - /* This is still better than hardcoded constant */ - for (size_t i = 0; i < sizeof(g_siphash_key); i++) { - g_siphash_key[i] = (uint8_t)(i ^ 0xAA); - } +static pthread_once_t g_siphash_key_once = PTHREAD_ONCE_INIT; + +/* Initialize SipHash key with random data - called once by pthread_once */ +static void init_siphash_key(void) { + /* Generate random key - this is best effort, if it fails we use fallback */ + gm_result_void result = + gm_random_bytes(g_siphash_key, sizeof(g_siphash_key)); + if (GM_IS_ERR(result)) { + /* If random generation fails, use a deterministic fallback */ + /* This is still better than hardcoded constant */ + for (size_t i = 0; i < sizeof(g_siphash_key); i++) { + g_siphash_key[i] = (uint8_t)(i ^ 0xAA); } - g_siphash_key_initialized = true; + gm_error_free(GM_UNWRAP_ERR(result)); } } /* Hash function for hash tables using SipHash-2-4 */ gm_result_u32 gm_id_hash(gm_id_t identifier) { - /* Ensure key is initialized - this can fail if random generation fails */ - ensure_siphash_key_initialized(); - - /* Check if initialization actually succeeded */ - if (!g_siphash_key_initialized) { - return gm_err_u32( - GM_ERROR(GM_ERR_INVALID_STATE, "Failed to initialize SipHash key")); - } + /* Ensure key is initialized exactly once, thread-safe */ + pthread_once(&g_siphash_key_once, init_siphash_key); /* SipHash produces 8-byte output */ uint8_t hash_output[crypto_shorthash_siphash24_BYTES];