wdgzyjx/myxce
1
0
Fork 0
forked from lei.chen/myxce
Code Pull requests Activity

fix(controller): set c->running in cluster_run so HA primary streams joblist/workerlist

Browse source 
cluster_start (which set c->running) is orphaned on the HA myapp_run path, so
the running flag was never set on HA nodes — check_controller() treated the
primary as shutting down and zeroed joblist/workerlist reply chunks. Own the
flag lifecycle in cluster_run, symmetric with the c->running=0 on shutdown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-06-16 03:38:55 +00:00
parent a735a880ae
commit 646aa69aa8
3 changed files with 47 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

10
src/controller/cluster.c
View file

@ -1439,6 +1439,16 @@ int cluster_start(cluster_t* c)
int cluster_run(cluster_t* c)
{
/* c->running is the controller's liveness flag: 1 while the run loop is
* active, 0 once a shutdown request breaks the loop below. check_controller()
* (the streaming gate for joblist/workerlist replies in xce_reply_get) treats
* !running as "shutting down" and zeroes the reply chunk. The legacy
* single-node bring-up set this in cluster_start(), but cluster_start is
* orphaned (not on the HA myapp_run path), so it was never set on HA nodes —
* leaving joblist/workerlist permanently empty on the primary. Own the whole
* flag lifecycle here, symmetric with the `c->running = 0` on shutdown. */
c->running = 1;
for (;;) {
int stop;

8
src/lib/request.c
View file

@ -240,6 +240,14 @@ int xce_decode(io_packet_t* pkt, xce_request_t* req)
void init_reply_by_req(xce_reply_t* reply, xce_request_t* req)
{
/* Zero the whole reply first. Callers declare `xce_reply_t reply;` on the
* stack and only populate the fields they need (status / data_which / data),
* leaving the common trailer fields (redirect_host, redirect_port) untouched.
* The generated encode_RequestReply reads redirect_host unconditionally
* (gated only on != NULL), so a stale stack value there is dereferenced via
* strlen() — a heap-use-after-free when the garbage aliases a freed capnp
* segment. Clearing here makes every reply path safe by construction. */
memset(reply, 0, sizeof(*reply));
reply->seq = req->seq;
reply->opcode = req->opcode;
}

29
src/lib/testsuite/test_request.c
View file

@ -65,6 +65,34 @@ static void test_hostctrl_req_reject_negative_action(void)
TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
}
/* init_reply_by_req must fully zero the reply so the generated
* encode_RequestReply never reads a stale `redirect_host` pointer (and the
* other common trailer fields) off an uninitialized stack reply. Regression
* guard for the heap-use-after-free in encode_RequestReply / strlen seen on a
* NOP/state-report reply whose redirect_host was stack garbage. */
static void test_init_reply_by_req_zeroes_reply(void)
{
/* Poison the whole reply with non-zero bytes — mirrors an uninitialized
* stack reply whose redirect_host happens to be a (dangling) non-NULL. */
xce_reply_t reply;
xce_request_t req = { .seq = 42, .opcode = OPCode_nop };
memset(&reply, 0xFF, sizeof(reply));
init_reply_by_req(&reply, &req);
/* seq/opcode carried from the request */
TEST_ASSERT_EQUAL_UINT64(42, reply.seq);
TEST_ASSERT_EQUAL_INT(OPCode_nop, reply.opcode);
/* Trailer/common fields the encoder always reads must be cleared so a
* handler that only sets data_which/data cannot leak garbage onto the wire
* (or dereference a dangling redirect_host). */
TEST_ASSERT_NULL(reply.redirect_host);
TEST_ASSERT_EQUAL_UINT(0, reply.redirect_port);
TEST_ASSERT_EQUAL_INT(0, reply.status);
TEST_ASSERT_EQUAL_INT(0, reply.data_which);
}
void test_request(void)
{
printf("\n>> host control request validation tests\n\n");
@ -76,4 +104,5 @@ void test_request(void)
RUN_TEST(test_hostctrl_req_reject_tag);
RUN_TEST(test_hostctrl_req_reject_untag);
RUN_TEST(test_hostctrl_req_reject_negative_action);
RUN_TEST(test_init_reply_by_req_zeroes_reply);
}