Skip to content

Commit b1554b4

Browse files
committed
Fix use-after-free in IOCP ASIO system
Two related races in the IOCP token mechanism allowed the event to be freed while something still held a raw pointer to it. Race 1 — callback vs destroy: pony_asio_event_destroy freed the event immediately via POOL_FREE regardless of whether IOCP callbacks were still in flight on thread pool threads. A callback could check the token's dead flag, see the event alive, and then have the event freed and pool-recycled before accessing it. The freed memory gets recycled by the pool allocator, the callback reads garbage, and eventually the scheduler jumps through a corrupted function pointer (DEP violation). Race 2 — message vs destroy: when a callback passes the dead check and calls pony_asio_event_send, the message carries a raw asio_event_t* pointer. The callback then releases its refcount via iocp_destroy, which can free the event if it's the last releaser after destroy marked the token dead. The message is now in the actor's queue with a dangling pointer. When the actor processes it, _event_notify dereferences freed memory. Fix: the token refcount now tracks all outstanding references to the event, not just in-flight IOCP operations: - The event itself: refcount starts at 1 (was 0). Destroy releases this reference instead of freeing the event directly. - Each in-flight IOCP operation: incremented when posted, decremented when the callback completes (unchanged). - Each in-flight message: incremented in pony_asio_event_send, decremented in handle_message after the behavior dispatch returns. Whoever decrements the refcount to zero (whether destroy, the last callback, or the last message dispatch) frees both the event and the token. The event stays alive until every callback has finished and every message has been processed. The previous token mechanism (dead flag + refcount) was designed to prevent exactly this class of bug but had TOCTOU flaws: checking dead and then accessing the event was not atomic, and the message pointer was not covered by the refcount at all. Surfaced by the Windows TCP open/close stress test (run https://github.com/ponylang/ponyc/actions/runs/23730792545/job/69123934918) — release-compiled + --ponynoblock only, ~1 in 10 daily runs. Closes #5092
1 parent 0033898 commit b1554b4

File tree

5 files changed

+67
-13
lines changed

5 files changed

+67
-13
lines changed

.release-notes/fix-iocp-toctou.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
## Fix use-after-free in IOCP ASIO system
2+
3+
We fixed a pair of use-after-free races in the Windows IOCP event system. A previous fix introduced a token mechanism to prevent IOCP callbacks from accessing freed events, but missed two windows where raw pointers could outlive the event they pointed to. One was between the callback and event destruction, the other between a queued message and event destruction.
4+
5+
This is the hard part that Pony protects you from. Concurrent access to mutable data across threads is genuinely difficult to get right, even when you have a mechanism designed specifically to handle it.

src/libponyrt/actor/actor.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#define PONY_WANT_ATOMIC_DEFS
22

33
#include "actor.h"
4+
#include "../asio/event.h"
45
#include "../sched/scheduler.h"
56
#include "../sched/cpu.h"
67
#include "../mem/pool.h"
@@ -472,6 +473,30 @@ static bool handle_message(pony_ctx_t* ctx, pony_actor_t* actor,
472473

473474
DTRACE3(ACTOR_MSG_RUN, (uintptr_t)ctx->scheduler, (uintptr_t)actor, msg->id);
474475
actor->type->dispatch(ctx, actor, msg);
476+
477+
#ifdef PLATFORM_IS_WINDOWS
478+
// Release the IOCP message reference taken in pony_asio_event_send.
479+
// The event pointer in the message must stay alive through dispatch;
480+
// now that the behavior has returned, we can release it.
481+
if(actor->type->event_notify != (uint32_t)-1 &&
482+
msg->id == actor->type->event_notify)
483+
{
484+
asio_msg_t* am = (asio_msg_t*)msg;
485+
asio_event_t* ev = am->event;
486+
iocp_token_t* token = ev->iocp_token;
487+
488+
if(atomic_fetch_sub_explicit(&token->refcount, 1,
489+
memory_order_acq_rel) == 1)
490+
{
491+
if(atomic_load_explicit(&token->dead, memory_order_acquire))
492+
{
493+
POOL_FREE(asio_event_t, ev);
494+
POOL_FREE(iocp_token_t, token);
495+
}
496+
}
497+
}
498+
#endif
499+
475500
return true;
476501
}
477502
}

src/libponyrt/asio/event.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ PONY_API asio_event_t* pony_asio_event_create(pony_actor_t* owner, int fd,
3535
#ifdef PLATFORM_IS_WINDOWS
3636
ev->iocp_token = POOL_ALLOC(iocp_token_t);
3737
atomic_store_explicit(&ev->iocp_token->dead, false, memory_order_relaxed);
38-
atomic_store_explicit(&ev->iocp_token->refcount, 0, memory_order_relaxed);
38+
// Start at 1: the event itself holds a reference. IOCP operations add more.
39+
// Destroy releases the event's reference; callbacks release theirs.
40+
// The last releaser (refcount 1 -> 0) frees both the event and the token.
41+
atomic_store_explicit(&ev->iocp_token->refcount, 1, memory_order_relaxed);
3942
#endif
4043

4144
owner->live_asio_events = owner->live_asio_events + 1;
@@ -72,7 +75,6 @@ PONY_API void pony_asio_event_destroy(asio_event_t* ev)
7275
ev->flags = ASIO_DESTROYED;
7376

7477
#ifdef PLATFORM_IS_WINDOWS
75-
// Grab the token pointer before freeing the event.
7678
iocp_token_t* token = ev->iocp_token;
7779

7880
// Mark the token as dead. Any IOCP callback that hasn't yet checked the
@@ -90,13 +92,18 @@ PONY_API void pony_asio_event_destroy(asio_event_t* ev)
9092
pony_assert(ev->owner->live_asio_events > 0);
9193
ev->owner->live_asio_events = ev->owner->live_asio_events - 1;
9294

93-
POOL_FREE(asio_event_t, ev);
94-
9595
#ifdef PLATFORM_IS_WINDOWS
96-
// Free the token if no IOCP callbacks are outstanding. If callbacks are
97-
// still in flight, the last one to complete will free the token.
98-
if(atomic_load_explicit(&token->refcount, memory_order_acquire) == 0)
96+
// Release the event's reference (created with refcount 1). If all IOCP
97+
// callbacks have already completed, we're the last releaser and free the
98+
// event and token. Otherwise the last callback to complete will free them.
99+
if(atomic_fetch_sub_explicit(&token->refcount, 1,
100+
memory_order_acq_rel) == 1)
101+
{
102+
POOL_FREE(asio_event_t, ev);
99103
POOL_FREE(iocp_token_t, token);
104+
}
105+
#else
106+
POOL_FREE(asio_event_t, ev);
100107
#endif
101108
}
102109

@@ -162,6 +169,13 @@ PONY_API void pony_asio_event_send(asio_event_t* ev, uint32_t flags,
162169
// scheduler index if this is run on a normal scheduler thread and that would
163170
// be not good.
164171
pony_register_thread();
172+
173+
// Hold a token reference for the message. The event pointer stored in the
174+
// message must remain valid until the actor finishes processing it.
175+
// The corresponding release happens in handle_message (actor.c) after
176+
// the behavior dispatch returns.
177+
atomic_fetch_add_explicit(&ev->iocp_token->refcount, 1,
178+
memory_order_relaxed);
165179
#endif
166180

167181
asio_msg_t* m = (asio_msg_t*)pony_alloc_msg(POOL_INDEX(sizeof(asio_msg_t)),

src/libponyrt/asio/event.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@ PONY_EXTERN_C_BEGIN
1414
* IOCP completion callbacks fire on Windows thread pool threads after the
1515
* owning actor may have destroyed the event. Each in-flight IOCP operation
1616
* holds a pointer to this token. The callback checks the dead flag before
17-
* touching the event; the refcount tracks how many callbacks are outstanding
18-
* so the token itself can be freed when no longer needed.
17+
* touching the event.
18+
*
19+
* The refcount starts at 1 (the event's own reference). Each IOCP operation
20+
* adds 1; each callback completion subtracts 1; destroy subtracts the
21+
* event's 1 and marks dead. Whoever decrements to zero frees both the
22+
* event and the token. This ensures the event stays alive until all
23+
* callbacks have finished.
1924
*/
2025
typedef struct iocp_token_t
2126
{

src/libponyrt/lang/socket.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,24 +228,28 @@ static iocp_t* iocp_create(iocp_op_t op, asio_event_t* ev)
228228
return iocp;
229229
}
230230

231-
static void iocp_release_token(iocp_token_t* token)
231+
static void iocp_release_token(iocp_token_t* token, asio_event_t* ev)
232232
{
233233
if(atomic_fetch_sub_explicit(&token->refcount, 1, memory_order_acq_rel) == 1)
234234
{
235235
// We were the last outstanding operation. If the event has been destroyed,
236-
// nobody else will free the token — we do it.
236+
// nobody else will free the event and token — we do it.
237237
if(atomic_load_explicit(&token->dead, memory_order_acquire))
238+
{
239+
POOL_FREE(asio_event_t, ev);
238240
POOL_FREE(iocp_token_t, token);
241+
}
239242
}
240243
}
241244

242245
static void iocp_destroy(iocp_t* iocp)
243246
{
244247
iocp_token_t* token = iocp->token;
248+
asio_event_t* ev = iocp->ev;
245249
POOL_FREE(iocp_t, iocp);
246250

247251
if(token != NULL)
248-
iocp_release_token(token);
252+
iocp_release_token(token, ev);
249253
}
250254

251255
static iocp_accept_t* iocp_accept_create(SOCKET s, asio_event_t* ev)
@@ -265,10 +269,11 @@ static iocp_accept_t* iocp_accept_create(SOCKET s, asio_event_t* ev)
265269
static void iocp_accept_destroy(iocp_accept_t* iocp)
266270
{
267271
iocp_token_t* token = iocp->iocp.token;
272+
asio_event_t* ev = iocp->iocp.ev;
268273
POOL_FREE(iocp_accept_t, iocp);
269274

270275
if(token != NULL)
271-
iocp_release_token(token);
276+
iocp_release_token(token, ev);
272277
}
273278

274279
static void CALLBACK iocp_callback(DWORD err, DWORD bytes, OVERLAPPED* ov)

0 commit comments

Comments
 (0)