From 0d783b16e3deeb7a1f4d69954c14e7ce786b491e Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 20 May 2026 08:53:38 +0000 Subject: [PATCH] fix(ha): zero ha_state_t before validation; fix invalid config in stale test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: test_heartbeat_stale_no_crash_without_primary had heartbeat_timeout_ms=100 < heartbeat_interval_ms=3000, causing ha_init to return -EINVAL before memset. The test ignored the return value and called ha_fini on garbage stack memory. The watchdog_thread field happened to hold the value 3 (from a prior stack frame), so ha_stop_watchdog called pthread_join(3, NULL) — crash at 0x2d3 (__pthread_clockjoin_ex accessing thread-struct field at offset 0x2d0). Fix 1 (test): valid config (interval=2000, timeout=3000). Staleness is injected via last_heartbeat -= 10s, so timeout value was irrelevant. Added TEST_ASSERT_EQUAL_INT check on ha_init return. Fix 2 (ha_init): move memset before validation so ha_fini is safe to call even after a failed init — prevents the same bug class in callers that check ha_init's return late. Verified: test_ha_role 11/11 PASS in both non-ASAN and ASAN builds. --- src/controller/ha.c | 5 +++-- src/controller/testsuite/test_ha_role.c | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/controller/ha.c b/src/controller/ha.c index d2d878cc..aa7f84ea 100644 --- a/src/controller/ha.c +++ b/src/controller/ha.c @@ -56,14 +56,15 @@ int ha_init(ha_state_t *ha, const ha_config_t *cfg, int my_priority) if (!ha || !cfg) return -EINVAL; + /* Zero first so ha_fini() is safe even if validation fails below */ + memset(ha, 0, sizeof(*ha)); + if (cfg->heartbeat_interval_ms < 2000) return -EINVAL; if (cfg->heartbeat_timeout_ms < cfg->heartbeat_interval_ms) return -EINVAL; if (cfg->election_timeout_ms < 500) return -EINVAL; - - memset(ha, 0, sizeof(*ha)); ha->config = *cfg; ha->my_priority = my_priority; diff --git a/src/controller/testsuite/test_ha_role.c b/src/controller/testsuite/test_ha_role.c index 33e2036d..184870cd 100644 --- a/src/controller/testsuite/test_ha_role.c +++ b/src/controller/testsuite/test_ha_role.c @@ -138,13 +138,14 @@ static void test_heartbeat_stale_no_crash_without_primary(void) { ha_state_t ha; ha_config_t cfg = { - .heartbeat_interval_ms = 3000, - .heartbeat_timeout_ms = 100, /* very short for test */ + .heartbeat_interval_ms = 2000, /* minimum valid */ + .heartbeat_timeout_ms = 3000, /* >= interval */ .election_timeout_ms = 2000, .probe_connect_timeout_ms = 500, .max_promotion_retries = 5, }; - ha_init(&ha, &cfg, 0); + /* Staleness is injected manually below; timeout value only needs to be valid */ + TEST_ASSERT_EQUAL_INT(0, ha_init(&ha, &cfg, 0)); /* Set last_heartbeat to a time far in the past to simulate timeout */ clock_gettime(CLOCK_MONOTONIC, &ha.last_heartbeat); ha.last_heartbeat.tv_sec -= 10; /* 10 seconds ago */