Valkey Over RDMA: server UAF crash in tests

Table of Contents

https://github.com/valkey-io/valkey/issues/3345

Main symptom: segfault on Ctrl+C — how do we fix it?

Our PR:

https://github.com/valkey-io/valkey/pull/3448

To explain this UAF server crash clearly in interviews, we need a careful postmortem. This is another Valkey Over RDMA series issue:

Background: why RDMA needs a pending list
#

RDMA connections have no TCP-style POLLOUT event to drive the write handler, so the server manually “wakes” connections via a pending list. Data movement uses one-sided RDMA with WRITE_WITH_IMM for notifications: we know when data is readable, but not when we can write:

/* RDMA connection is always writable, it has no POLLOUT event to drive the write handler, record available write
 * handler into pending list */
static list *pending_list;

There is no way to trigger a write callback via POLLOUT.

Each rdma_connection has a pending_list_node — both a list node pointer and a “already on the list?” flag:

typedef struct rdma_connection {
    connection c;
    struct rdma_cm_id *cm_id;
    int flags;
    int last_errno;
    listNode *pending_list_node; // 这里应该是指向最后的节点
} rdma_connection;

The event loop handles it in beforeSleep():

    /* Handle pending data(typical TLS). (must be done before flushAppendOnlyFile) */
    int conn_pending = connTypeProcessPendingData();
    if (conn_pending > 0) server.el_iteration_active = true;

→ rdmaProcessPendingData() walks pending_list; for CLOSED connections it invokes read/write handlers to finish cleanup.

Connections that need work are hung on this list for unified processing.

Bug: the same connection is added to `pending_list` twice
#

If you free twice, that is UAF — walk the chain carefully; it is nasty.

Trigger
#

valkey-benchmark --rdma -c 256, interrupt with Ctrl+C → 256 RDMA connections disconnect almost at once.

Timeline (before the fix)
#

T1  RDMA CM 事件: DISCONNECTED  -》 此时发生RDMA断连
    rdmaHandleDisconnect()
      conn->state = CONN_STATE_CLOSED -》 注意，这个时候修改了连接的状态为CLOSED
      listAddNodeTail(pending_list, conn)     ← 第 1 次入队，node_A -> NULL
      pending_list_node = node_A

T2  同一连接上，write handler 被更新（仍有 pending write）
    connRdmaSetWriteHandler(func=非NULL)
      listAddNodeTail(pending_list, conn)     ← 第 2 次入队，node_B
      pending_list_node = node_B              ← 加在了node_A后面！

你可以看到这样的状态，同时 conn->state = CONN_STATE_CLOSED

pending_list: [ node_A → conn ] → [ node_B → conn ]
              同一个 conn 出现两次，但 pending_list_node 只记住 B

So first node_A (connection needs close), then node_B (connection needs write handling). The enqueue conditions were too loose.

What is a CM event?

CM events are asynchronous connection-lifecycle notifications from the RDMA Connection Manager (librdmacm) to the application.

In Valkey they own the control plane for connect/disconnect;

The data plane that actually moves bytes is QP + CQ — a different fd and event path.

┌─────────────────────────────────────────────┐
│  应用层  Valkey server / libvalkey client    │
├─────────────────────────────────────────────┤
│  CM 层   librdmacm  (rdma_connect/accept)   │  ← CM 事件在这里,怪不得我不知道
├─────────────────────────────────────────────┤
│  Verbs   libibverbs (ibv_post_send/recv...) │  ← CQ 完成事件在这里
├─────────────────────────────────────────────┤
│  内核    rdma_cm 模块 + ib_uverbs 字符设备     │
├─────────────────────────────────────────────┤
│  硬件    HCA (Host Channel Adapter)          │
└─────────────────────────────────────────────┘

This layer mainly manages connections — here, the CM disconnect event.

Worth studying further. A CM event is struct rdma_cm_event, produced by the kernel rdma_cm subsystem and fetched in userspace via rdma_get_cm_event():

struct rdma_cm_event {
    enum rdma_cm_event_type event;  // 事件类型
    struct rdma_cm_id *id;          // 关联的连接标识
    ...
};

Crash path (pre-fix `rdmaProcessPendingData`)
#

Pre-fix logic (parent of 75fee11c6):

// 旧代码 — 已不存在于当前树 这个时候预期遍历的是node_A
if (conn->state == CONN_STATE_CLOSED) { // 如果当前连接状态结束,需要删除这个节点
    listDelNode(pending_list, rdma_conn->pending_list_node);  // 删的是 node_B
    rdma_conn->pending_list_node = NULL;
    if (callHandler(conn, conn->read_handler)) {            // 正在遍历 node_A
        callHandler(conn, conn->write_handler);
    }
    ...
}

First iteration (ln = node_A):

pending_list_node points at node_B (overwritten at T2)
listDelNode removes node_B; node_A stays on the list
callHandler → readQueryFromClient → handleReadResult → freeClientAsync(c) (async free)

    if (c->nread <= 0) {
        if (c->nread == -1) {
            if (connGetState(c->conn) != CONN_STATE_CONNECTED) {
                ...
                freeClientAsync(c);
            }
        } else if (c->nread == 0) {
            ...
            freeClientAsync(c);
        }
        return C_ERR;

freeClientAsync only enqueues — it does not immediately zfree(conn):

void freeClientAsync(client *c) {
    if (c->flag.close_asap || c->flag.script) return;
    c->flag.close_asap = 1;
    ...
    listAddNodeTail(server.clients_to_close, c);
}

Real freeClient → connClose → releases rdma_connection in freeClientsInAsyncFreeQueue(), after rdmaProcessPendingData() returns:

        processed += freeClientsInAsyncFreeQueue();

The fake second entry (node_B) causes the connection to be actually freed.

Second iteration (ln = node_A, still on the list):

listNodeValue(ln) yields the same conn, already marked closed / mid-teardown
callHandler again → handleReadResult touches c->nread etc.
If memory was reused or client/conn is inconsistent → SIGSEGV @ 0x8 (NULL + offset 8 — classic UAF)

First pass via node_B freed the connection; processing node_A frees/uses it again.

handleReadResult
  ← readQueryFromClient
    ← rdmaProcessPendingData
      ← beforeSleep

Fix: four changes, one invariant
#

Core invariant: each connection appears in pending_list at most once; on delete, remove the current iteration node ln, do not trust pending_list_node (it may have been overwritten).

Fix 1: `rdmaHandleDisconnect` — check before enqueue
#

    /* we can't close connection now, let's mark this connection as closed state */
    if (rdma_conn->pending_list_node == NULL) {
        listAddNodeTail(pending_list, conn);
        rdma_conn->pending_list_node = listLast(pending_list);
    }

If pending_list_node == NULL: the slot is empty and the conn may truly enqueue.

Fix 2: `connRdmaSetWriteHandler` — same duplicate guard
#

    /* does this connection has pending write data? */
    if (func) {
        if (rdma_conn->pending_list_node == NULL) {
            listAddNodeTail(pending_list, conn);
            rdma_conn->pending_list_node = listLast(pending_list);
        }
    } else if (rdma_conn->pending_list_node) {
        listDelNode(pending_list, rdma_conn->pending_list_node);
        rdma_conn->pending_list_node = NULL;
    }

On disconnect, if a write handler is still set, old code unconditionally enqueued again — a main source of duplicates. Even when setting the handler, check first.

Fix 3 + 4: `rdmaProcessPendingData` — delete via `ln` + handlers before unlink
#

Current logic:

static int rdmaProcessPendingData(void) {
    listIter li;
    listNode *ln;
    rdma_connection *rdma_conn;
    connection *conn;
    int processed = 0;

    listRewind(pending_list, &li);
    while ((ln = listNext(&li))) {
        rdma_conn = listNodeValue(ln);
        if (rdma_conn->flags & RDMA_CONN_FLAG_POSTPONE_UPDATE_STATE) continue;
        conn = &rdma_conn->c;

        /* a connection can be disconnected by remote peer, CM event mark state as CONN_STATE_CLOSED, kick connection
         * read/write handler to close connection */
        if (conn->state == CONN_STATE_ERROR || conn->state == CONN_STATE_CLOSED) {
            /* Invoke both read_handler and write_handler, unless read_handler
               returns 0, indicating the connection has closed, in which case
               write_handler will be skipped. */
            if (callHandler(conn, conn->read_handler)) {
                callHandler(conn, conn->write_handler);
            }

            listDelNode(pending_list, ln);
            rdma_conn->pending_list_node = NULL;

            ++processed;
            continue;
        }

        connRdmaEventHandler(NULL, -1, rdma_conn, 0);
        ++processed;
    }

    return processed;
}

sequenceDiagram
    participant CM as RDMA CM Event
    participant Disc as rdmaHandleDisconnect
    participant WH as connRdmaSetWriteHandler
    participant PL as pending_list
    participant BSP as beforeSleep
    participant PPD as rdmaProcessPendingData
    participant RH as readQueryFromClient

    CM->>Disc: DISCONNECTED
    Disc->>PL: add conn (if pending_list_node==NULL)
    Note over PL: node_A

    WH->>PL: add conn again (每次都会加上，问题就出在这里，所以crash是必现)
    Note over PL: node_B, pending_list_node=B

    BSP->>PPD: connTypeProcessPendingData()
    PPD->>PPD: iterate ln=node_A
    Note over PPD: OLD: delete node_B, handler, node_A remains
    PPD->>RH: callHandler (2nd iter on node_A)
    Note over RH: UAF / SIGSEGV @ 0x8

    Note over PPD: NEW: guard duplicates + delete ln + handler first

Background: why RDMA needs a pending list #

Bug: the same connection is added to pending_list twice #

Trigger #

Timeline (before the fix) #

Crash path (pre-fix rdmaProcessPendingData) #

Fix: four changes, one invariant #

Fix 1: rdmaHandleDisconnect — check before enqueue #

Fix 2: connRdmaSetWriteHandler — same duplicate guard #

Fix 3 + 4: rdmaProcessPendingData — delete via ln + handlers before unlink #