lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(lib): keep a refused inbound packet endpoint-owned instead of leaking it (bug-1918)

Browse source 
INVOKE_MSG_HANDLER stored NULL into ep->fsm_in.pkt after every msg
handler. Handlers that take the packet clear the slot themselves
(vtask_handle_msg, peer_ctrl_msg_handler); one that refuses it
(-EINVAL / -ENOMEM) leaves it there, and the NULL store dropped the
only reference. The slot is now left as the handler leaves it: a
refused packet is reused for the next message or freed by endpoint_fini.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-10-04 23:59:29 +00:00
parent e27bc2ed3a
commit a74b684c48
Signed by untrusted user: rshen
GPG key ID: 80533536B0EB651C
2 changed files with 66 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

11
src/include/netio.h
View file

@ -141,8 +141,8 @@ typedef struct endpoint_t {
* via the vtask dispatcher, so the vtask_enq/-1 ref pair does not cover this
* call). A handler whose own logic drops ep's last reference (e.g. a stale
* vote reply driving raft_cb_become_follower -> endpoint_unref on the very ep
* currently being read) frees ep out from under the trailing pkt=NULL write
* below, a heap-use-after-free. Hold a ref for the duration of the call, same
* currently being read) frees ep out from under the macro's own trailing use
* of it, a heap-use-after-free. Hold a ref for the duration of the call, same
* pattern as the vtask dispatch path, so the handler's own unref cannot drop
* the count to zero until this macro is done touching ep. */
#define INVOKE_MSG_HANDLER(ep) \
@ -150,9 +150,12 @@ typedef struct endpoint_t {
TRACE("%s msg handler %p", __func__, (ep)->msg_handler); \
if ((ep)->msg_handler != NULL) { \
endpoint_ref(ep); \
/* A handler that takes the packet clears ep->fsm_in.pkt itself \
* (vtask_handle_msg, peer_ctrl_msg_handler). One that refuses it \
* (early -EINVAL/-ENOMEM) leaves it endpoint-owned: the next \
* message reuses it and endpoint_fini frees it. Overwriting the \
* slot here leaked every refused packet (bug-1918). */ \
(ep)->msg_handler(ep); \
/* passing ownership of packet to upper layer */ \
(ep)->fsm_in.pkt = NULL; \
endpoint_unref(ep); \
} \
} while (0)

59
src/lib/testsuite/test_vtask_pkt_handoff.c
View file

@ -189,9 +189,68 @@ static void test_handoff_rejects_incomplete_fsm(void)
vtaskmgr_fini(&mgr);
}
/* bug-1918: a msg handler that refuses the packet (an early -EINVAL/-ENOMEM
* exit, e.g. handle_msg with no owner context or vtask_handle_msg failing to
* allocate its task) leaves it in ep->fsm_in.pkt. INVOKE_MSG_HANDLER used to
* overwrite the slot with NULL after every call, leaking that packet; it must
* stay endpoint-owned so the next message reuses it or endpoint_fini frees it.
* A handler that takes the packet clears the slot itself (vtask_handle_msg,
* peer_ctrl_msg_handler), so the macro has nothing to clear. */
static int refusing_msg_handler(endpoint_t* ep)
{
(void)ep;
return -ENOMEM;
}
static int taking_msg_handler(endpoint_t* ep)
{
io_packet_t* pkt = ep->fsm_in.pkt;
ep->fsm_in.pkt = NULL;
free_packet(pkt);
free(pkt);
return 0;
}
static void test_refused_packet_stays_endpoint_owned(void)
{
endpoint_t ep;
io_packet_t* pkt;
memset(&ep, 0, sizeof(ep));
TEST_ASSERT_TRUE(endpoint_init(&ep, AF_INET, SOCK_STREAM, 0, 0) >= 0);
pkt = encode_nop();
arm_inbound_done(&ep, pkt);
ep.msg_handler = (msg_handler_t)refusing_msg_handler;
INVOKE_MSG_HANDLER(&ep);
TEST_ASSERT_EQUAL_PTR_MESSAGE(
pkt, ep.fsm_in.pkt,
"bug-1918: a packet the handler refused must stay in the endpoint "
"slot (it was dropped from the slot and leaked)");
/* A handler that takes the packet leaves the slot empty. */
ep.fsm_in.pkt = NULL;
free_packet(pkt);
free(pkt);
arm_inbound_done(&ep, encode_nop());
ep.msg_handler = (msg_handler_t)taking_msg_handler;
INVOKE_MSG_HANDLER(&ep);
TEST_ASSERT_NULL(ep.fsm_in.pkt);
/* Refused again: endpoint_fini frees the endpoint-owned packet. */
arm_inbound_done(&ep, encode_nop());
ep.msg_handler = (msg_handler_t)refusing_msg_handler;
INVOKE_MSG_HANDLER(&ep);
TEST_ASSERT_NOT_NULL(ep.fsm_in.pkt);
endpoint_fini(&ep);
TEST_ASSERT_NULL(ep.fsm_in.pkt);
}
void test_vtask_pkt_handoff(void)
{
RUN_TEST(test_endpoint_fini_after_vtask_freed_pkt);
RUN_TEST(test_handoff_detaches_before_task_runs);
RUN_TEST(test_handoff_rejects_incomplete_fsm);
RUN_TEST(test_refused_packet_stays_endpoint_owned);
}