Hello, Hongyan,
Sorry for the delay, and thanks for the patch.
On Fri, Sep 04, 2026 at 11:31:30AM +0800, Hongyan Xu wrote:
> The destination RDMA accept path runs the incoming migration coroutine
> (mis->loadvm_co). While blocked waiting for an RDMA completion it parks
> itself in yield_until_fd_readable() (util/qemu-coroutine-io.c) on a
> completion-channel fd. That helper registers an fd handler whose opaque
> is a stack object (FDYieldUntilData) and only removes the handler from
> inside its own fd_coroutine_enter() callback.
>
> rdma_cm_poll_handler() handles RDMA_CM_EVENT_DISCONNECTED /
> RDMA_CM_EVENT_DEVICE_REMOVAL by calling qemu_coroutine_enter() on the
> coroutine directly. When the coroutine is parked in
> yield_until_fd_readable() at that moment, this bypasses
> fd_coroutine_enter(): the coroutine resumes and returns from the wait,
> but the fd handler stays registered with an opaque that points into the
> (now returned-from) stack frame. A later event on that fd then calls
> fd_coroutine_enter() with a dangling pointer -> use-after-free.
Looks like an issue indeed, and I wonder if it's a generic coroutine api
problem. More below.
>
> Record, around the yield, the AioContext and fd the coroutine is parked
> on, and make rdma_cm_poll_handler() remove that fd handler before it
> re-enters the coroutine.
>
> This is an RFC: the coroutine/thread wake-up semantics in this area were
> also discussed by Peter Xu's 2025 series ("migration/rdma: Remove
> coroutine path in qemu_rdma_wait_comp_channel" / "Remove
> rdma_cm_poll_handler"); if that direction is preferred this patch should
> be reworked accordingly.
I assume you meant this:
https://lore.kernel.org/qemu-devel/[email protected]/
I kept getting interrupted by all the things so that series was stranded
for now.. I'll higher priority of this series.. if someone is willing to
pick that work up before I do, please let me know. It's always good to fix
problems by removing code..
>
> Signed-off-by: Hongyan Xu <[email protected]>
> ---
> migration/rdma.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index e976739fad..66175aa60b 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -360,6 +360,17 @@ typedef struct RDMAContext {
> */
> int migration_started_on_destination;
>
> + /*
> + * While the incoming-migration coroutine is parked in
> + * yield_until_fd_readable() on a completion channel we record the
> + * fd and AioContext here. The cm event handler uses this to remove
> + * the fd handler before re-entering the coroutine; without that the
> + * handler keeps pointing at the coroutine stack after the wait
> + * returns, i.e. a dangling fd handler.
> + */
> + AioContext *wait_ctx;
> + int wait_fd;
> +
> int total_registrations;
> int total_writes;
>
> @@ -1248,7 +1259,21 @@ qemu_rdma_wait_comp_channel(RDMAContext *rdma,
> struct rdma_cm_event *cm_event;
>
> if (qemu_in_coroutine()) {
> + AioContext *ctx = qemu_get_current_aio_context();
> +
> + /*
> + * Record where the coroutine is parked so that
> + * rdma_cm_poll_handler() can remove the fd handler before it
> + * re-enters us (yield_until_fd_readable() only removes the
> + * handler through its own fd_coroutine_enter() callback; a
> + * direct qemu_coroutine_enter() would leave a handler whose
> + * opaque points at our stack frame).
> + */
> + rdma->wait_ctx = ctx;
> + rdma->wait_fd = comp_channel->fd;
> yield_until_fd_readable(comp_channel->fd);
> + rdma->wait_ctx = NULL;
> + rdma->wait_fd = -1;
> } else {
> /* This is the source side, we're in a separate thread
> * or destination prior to migration_fd_process_incoming()
> @@ -3024,6 +3049,21 @@ static void rdma_cm_poll_handler(void *opaque)
> }
> rdma_ack_cm_event(cm_event);
> if (mis->loadvm_co) {
> + /*
> + * The incoming coroutine may be parked in
> + * yield_until_fd_readable() on a completion channel. Its
> + * fd handler is normally removed by fd_coroutine_enter()
> + * when that fd becomes readable. If we re-enter the
> + * coroutine directly we must remove the handler first,
> + * otherwise it stays registered with an opaque pointing at
> + * the (now returned-from) coroutine stack frame.
> + */
> + if (rdma->wait_fd >= 0 && rdma->wait_ctx) {
> + aio_set_fd_handler(rdma->wait_ctx, rdma->wait_fd,
> + NULL, NULL, NULL, NULL, NULL);
> + rdma->wait_fd = -1;
> + rdma->wait_ctx = NULL;
> + }
> qemu_coroutine_enter(mis->loadvm_co);
> }
> return;
> --
> 2.50.1.windows.1
>
I feel like this is a bit of an overkill.
If the problem is about wrongly leftover of fd handlers, can we always
unregister it, like this?
Copy more people (both rdma and coroutines).
===8<===
diff --git a/util/qemu-coroutine-io.c b/util/qemu-coroutine-io.c
index 364f4d5abf..7bf38f4495 100644
--- a/util/qemu-coroutine-io.c
+++ b/util/qemu-coroutine-io.c
@@ -74,7 +74,7 @@ typedef struct {
static void fd_coroutine_enter(void *opaque)
{
FDYieldUntilData *data = opaque;
- aio_set_fd_handler(data->ctx, data->fd, NULL, NULL, NULL, NULL, NULL);
+ /* Postpone fd handler removal to be within the coroutine */
qemu_coroutine_enter(data->co);
}
@@ -89,4 +89,9 @@ void coroutine_fn yield_until_fd_readable(int fd)
aio_set_fd_handler(data.ctx, fd, fd_coroutine_enter, NULL, NULL, NULL,
&data);
qemu_coroutine_yield();
+ /*
+ * Always remove the fd handler, no matter the re-entering of this
+ * coroutine was caused by fd_coroutine_enter() or something else.
+ */
+ aio_set_fd_handler(data.ctx, fd, NULL, NULL, NULL, NULL, NULL);
}
--
Peter Xu
