Skip to content

ggml-rpc : bound rdma_recv completion length before copy - #24136

Closed
palios-taey wants to merge 1 commit into
ggml-org:masterfrom
palios-taey:fix/rpc-rdma-recv-bound
Closed

ggml-rpc : bound rdma_recv completion length before copy#24136
palios-taey wants to merge 1 commit into
ggml-org:masterfrom
palios-taey:fix/rpc-rdma-recv-bound

Conversation

@palios-taey

Copy link
Copy Markdown
Contributor

Found this fuzzing the RPC RDMA transport. socket_t::impl::rdma_recv (ggml/src/ggml-rpc/transport.cpp:450) copies wc.byte_len bytes into the caller's buffer with no check that it fits the requested size, then does rem -= got — a size_t underflow. The completion length is peer-controlled (up to the posted recv-slot size), so a peer that sends more than the logical recv expects overflows the destination and turns the loop into an unbounded copy; a zero-length completion makes no progress and spins.

Hit it as an ASAN stack-buffer-overflow WRITE on real RoCEv2 hardware with a single oversized post-HELLO SEND. DoS is confirmed (deterministic abort); I have not demonstrated control-transfer, so I'm not claiming RCE — just a peer-reachable out-of-bounds write.

The fix bounds got before the copy (kills the OOB write, the rem -= got underflow, and a 0-length spin) and drops the connection rather than clamping, so a malformed peer can't silently desync the message stream. Re-confirmed unpatched on master before filing; fix-validated on the same hardware (the oversized SEND now fails closed).

This is the experimental RPC backend, which SECURITY.md places outside the covered threat model — so I'm filing it as a robustness fix, not a security-scope claim. Full PoC + a SoftRoCE repro available on request.

AI usage disclosure: AI-assisted in the RDMA fuzzing that surfaced this and in drafting the fix; I validated the overflow and the fix on real hardware myself and own the change.

socket_t::impl::rdma_recv copies wc.byte_len bytes into the caller buffer
with no check that it fits the requested size, then does rem -= got
(size_t underflow). The completion length is peer-controlled (up to the
posted recv-slot size), so a peer sending more than the logical recv
expects overflows dst and turns the loop into an unbounded copy; a
zero-length completion makes no progress and spins. Reject both and drop
the connection rather than clamp, so a malformed peer cannot silently
desync the message stream.

Surfaced as an ASAN stack-buffer-overflow WRITE on real RoCEv2 hardware
via a single oversized post-HELLO SEND; the guard makes it fail closed.
@palios-taey
palios-taey requested a review from a team as a code owner June 4, 2026 18:11
@github-actions github-actions Bot added the ggml changes relating to the ggml tensor library for machine learning label Jun 4, 2026
@ggml-gh-bot

ggml-gh-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown

Hi @palios-taey, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

  • AI-generated content: This project does not accept PRs, descriptions or commit messages that are fully or predominantly AI-generated. If you have used AI to assist you in writing code, please make sure to disclose that explicitly.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@palios-taey

Copy link
Copy Markdown
Contributor Author

Closing to respect the new-contributor 1-open-PR limit (per the automated checker — apologies for the over-submission). Keeping #24093 (already approved) as my single open PR; I'll resubmit this after that merges. The change here is verified and stands on its own. Thanks!

@palios-taey palios-taey closed this Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant