Skip to content

[RDMA] stale PollCq callback can survive Reset/Revive and poll a new-generation CQ #3410

Description

@legionxiong

Describe the bug

An old RdmaEndpoint::PollCq callback can survive an RDMA
Reset()/health-check Revive() cycle and then access the resources of
the new RDMA generation.

PR #3261 / commit 58fec6f documented that a queued CQ callback may run
after the main Socket has been reset and revived. The merged fix drains
the old CQ after moving the QP to RESET, but it does not invalidate or
synchronize with a callback that has already copied the non-owning
RdmaEndpoint* from the old CQ Socket.

The following interleaving is still possible:

Old CQ Socket callback                 Main Socket / health checker
----------------------                 ----------------------------

PollCq(old_cq_socket)
  ep = old_cq_socket->user()
  [preempted before Address()]         QP failure / SetFailed()
                                       WaitAndReset()
                                       RdmaEndpoint::Reset()
                                         DeallocateResources()
                                           old_cq_socket->_user = NULL
                                           drain old CQ
                                       CheckHealth() / Revive()
                                       AllocateResources()
                                         create new _resource
                                         create new _cq_sid

  Socket::Address(ep->_socket->id())
    succeeds after revival

  cq = ep->_resource->...              New CQ callback may run
    // This is the new CQ
  ibv_poll_cq(cq)
  HandleCompletion(wc)

Setting the old CQ Socket's _user to NULL does not invalidate the
ep pointer already copied by a running callback.

PollCq verifies that the main Socket is addressable, but does not
verify that:

  • m->id() is still equal to ep->_cq_sid;
  • the callback belongs to the current RDMA generation;
  • ep->_resource is the resource for which the callback was created.

The old and new CQ Socket callbacks can therefore both poll the new CQ
and concurrently call HandleCompletion() on the same RdmaEndpoint.

Because they belong to different CQ Sockets, Socket-level event
serialization does not serialize their access to the endpoint.
HandleCompletion() modifies _sq_sent, _rq_received, _sbuf,
_rbuf, and the main Socket's _read_buf without endpoint-level
synchronization.

For example, concurrent SEND completions can race on:

_sbuf[_sq_sent++].clear();

This may result in an out-of-bounds _sbuf access or concurrent
IOBuf::clear() operations, matching the crash stack reported in
#3252:

butil::IOBuf::clear()
brpc::rdma::RdmaEndpoint::HandleCompletion()
brpc::rdma::RdmaEndpoint::PollCq()
brpc::Socket::ProcessEvent()

To Reproduce

There is no minimal standalone reproducer yet.

The race can be reproduced deterministically with a test hook:

  1. Enable RDMA and Socket health checking.
  2. Pause an old CQ callback immediately after reading m->user(), but
    before calling Socket::Address(ep->_socket->id(), ...).
  3. Force the QP into an error state.
  4. Allow WaitAndReset(), RdmaEndpoint::Reset(), health checking, and
    Revive() to complete.
  5. Complete a new RDMA handshake so that a new _resource and
    _cq_sid are installed.
  6. Resume the old callback while the new CQ callback is active.
  7. Generate SEND/RECV completions or run the test under TSAN.

Expected behavior

A CQ callback must never access an RdmaEndpoint or RdmaResource
belonging to a different Reset/Revive generation.

Reset should either wait for all callbacks of the old generation to
finish, or callbacks should carry lifetime-protected context and reject
themselves when their CQ Socket, resource, or generation is no longer
current.

A complete fix could bind each callback to immutable context such as:

{ main_socket_id, cq_socket_id, resource, generation }

After acquiring the main Socket, PollCq should validate the CQ Socket,
resource, and generation before accessing ep->_resource.
Polling-mode REMOVE operations require equivalent synchronization.

Versions
OS: Debian 12
Compiler: g++ 14.2.0
brpc: 1.14; the callback lifecycle described above is also present in master
protobuf: not relevant

Additional context/screenshots

Related issue: #3252

Related PR: #3261

CQ-draining fix: 58fec6fe

PR #3261 explicitly considered incrementing a generation in Reset()
and validating it in PollCq, but the merged implementation chose CQ
draining only.

Draining removes stale CQEs. It does not stop an old callback from
resuming after revival and selecting the newly installed
ep->_resource.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions