Open source · ai-dynamo/nixl · #2520

An integer overflow that bypassed NIXL's registration bounds

A strided run with stride 2⁶³ and count 3 spans 2⁶⁴ + 1 bytes. In 64-bit unsigned arithmetic that wraps to 1, a 1-byte span fits inside any registration, and the transfer reached memory nobody had registered.

Context: registration is the safety boundary

I fixed a memory-safety bug in NIXL (issue #2520), NVIDIA’s open-source library for fast point-to-point data transfer in inference frameworks such as Dynamo. NIXL moves data between CPU memory, GPU memory and storage through plug-in backends.

Applications register the memory they want to transfer, and they trust NIXL to read and write only inside those registrations. That check is the library’s memory-safety boundary. If a request can step over it, the result is a crash at best, and at worst data silently read from or written to the wrong place.

A strided run describes many evenly spaced blocks with one descriptor: a start address, a block length, a stride and a count. Before accepting a run, nixlMemSection::populate() computes the run’s full byte extent and checks that a registered region covers it:

const size_t span_len = (run.count - 1) * run.stride + run.len;

The walkthrough below plays the bug and the fix end to end. The sections after it go through each step in detail.

Animated explainer · 6 scenes · 49 s
0263264registeredtrue span: 264 + 1 bytesspan_len = 1span_len = 263 + 1addr + 264 wraps back to addr0122addr + 263I/O lands here: never registered✓ covered✗ rejected: INVALID_PARAM, no I/O✗ not covered: rejected as before
  1. { addr, len: 1, stride: 263, count: 3 }

    The request. A strided run describes count blocks of len bytes, spaced stride bytes apart. Block 0 starts at addr, inside the registered buffer.

  2. block i at addr + i × stride → addr, addr + 263, addr + 264 ≡ addr

    Where the blocks land. Block 1 is 263 bytes past addr, far outside the buffer. Block 2 is 264 bytes past addr, which wraps the 64-bit address space back around to addr.

  3. span_len = (3 − 1) × 263 + 1 = 264 + 1 → 1 in size_t

    The span wraps. Before any I/O, NIXL checks one span from the first block to the end of the last. The true span is 264 + 1 bytes, but size_t keeps only the low 64 bits, so span_len comes out as 1.

  4. covers(addr, span_len = 1) → true

    The check passes. A 1-byte span at addr fits inside the buffer, so the coverage check accepts the run, and the transfer touches addr + 263: memory nobody registered.

  5. count − 1 > (SIZE_MAX − len) / stride → 2 > 1 → NIXL_ERR_INVALID_PARAM

    The fix. The guard tests the same inequality by dividing instead of multiplying, so the check itself cannot wrap. 2 > 1, so the run is rejected before any memory is touched.

  6. count 2: 1 > 1 is false → span_len = 263 + 1 → covers() false

    Only overflow is rejected. With count 2 the span is 263 + 1 bytes, which fits in size_t. The guard lets it through, and the ordinary coverage check now sees the true size and rejects it as uncovered.

Root cause: a span that wraps to one byte

That line runs in 64-bit unsigned arithmetic with no overflow check. With count 3, stride 2⁶³ and length 1, the true span is 2⁶⁴ + 1 bytes, but the result wraps to 1. A 1-byte span fits inside any registration, so the coverage check passed, and makeXferReq then built descriptors outside the registered region. The transfer touched addr + 2⁶³. On the reporter’s machine that crashed the I/O. Had the address been mapped, it would have read or written memory with no error at all.

The issue’s strided run: before the fix the span wraps to 1 byte, passes the coverage check and reaches unregistered memory. After the fix the overflow guard rejects it with NIXL_ERR_INVALID_PARAM before any I/O

The same unchecked arithmetic appeared in two related places: the running flat block index (start_idx += run.count) and nixlMetaStrideDescList::flatSize(), which returns start_idx + count of the last run.

I traced the request path from prepXferDlist through populate() to the coverage check to confirm that this was the only place the wrapped value was accepted.

The fix: test the inequality without computing it

Rather than compute the span and then ask whether it overflowed, the guard rewrites the inequality so it never multiplies. (count - 1) * stride + len <= SIZE_MAX is equivalent to count - 1 <= (SIZE_MAX - len) / stride for positive stride, and the right-hand side uses only a subtraction that cannot underflow and a division that cannot overflow.

// Reject runs whose span or flat block count would overflow size_t.
if ((run.count - 1 > (SIZE_MAX - run.len) / run.stride) ||
    (run.count > SIZE_MAX - start_idx)) [[unlikely]] {
    resp.clear();
    return NIXL_ERR_INVALID_PARAM;
}
  • The first clause rejects a span that would overflow. For the issue’s input, (SIZE_MAX - 1) / 2⁶³ is 1, and count - 1 is 2, so the run is rejected.
  • The second clause rejects a run whose count would overflow the running block index.
  • A validation check just above already rejects count == 0, len == 0 and stride < len, so the division is always defined and count - 1 cannot underflow.

Rejected runs clear the partially built response and return NIXL_ERR_INVALID_PARAM, matching the error handling next to it. Callers of prepXferDlist see NIXL_ERR_NOT_FOUND, the same error as any other run that no registration covers.

flatSize() has no way to report an error, so there I added a NIXL_ASSERT(last.count <= SIZE_MAX - last.start_idx) that documents the invariant populate() now enforces.

I left one related wrap alone. nixlBasicDesc::covers() can overflow when addr + span wraps, but open PR #2142 already handles it, so touching it would only have created conflicting changes. I called it out for reviewers instead.

The change is 28 lines added and none removed across three files: src/infra/nixl_memory_section.cpp (the guard), src/api/cpp/backend/backend_aux.h (the assertion) and test/gtest/unit/agent/agent.cpp (the test).

Verification: differential fuzzing against 128-bit math

An overflow guard written with division is easy to get subtly wrong at the boundary, so I checked it against a reference where nothing can overflow. The reference computes the span in 128-bit integers. Across 20,000,000 random (count, stride, len) inputs, the guard rejected exactly the overflowing cases and nothing else, with zero mismatches.

Scenario Before After
Issue input: count 3, stride 2⁶³, len 1 Accepted; I/O reached addr + 2⁶³ Rejected with NIXL_ERR_INVALID_PARAM
Largest valid run at that stride: count 2 (span 2⁶³ + 1) Accepted Accepted
Ordinary strided runs Accepted Accepted; one extra integer division per run
Running block index overflow Unchecked Rejected
flatSize() total Unchecked Guarded by a debug assertion

Extreme values such as stride = SIZE_MAX also behave correctly.

The regression test, PrepStrideDlistRejectsOverflowingSpan, first prepares a valid strided run on the same buffer and asserts it succeeds, then submits the issue’s overflowing run and asserts it is rejected with no handle returned. The positive control matters: without it, a test that only checks rejection would also pass if the buffer setup were broken. Without the fix, the second half fails.

The PR’s CI checks, including clang-format and pre-commit, pass.

Core concepts this exercised

  • Unsigned overflow is silent. In C++, unsigned arithmetic wraps by definition rather than being undefined behavior, so neither the compiler nor the default UndefinedBehaviorSanitizer flags it. A bounds check built on a wrapping expression can be bypassed by choosing inputs that wrap to a small value.
  • Rewrite the check, not just the types. Dividing instead of multiplying turns an overflow-prone comparison into an equivalent one that cannot wrap, at the cost of one integer division per run.
  • Fail before touching memory. The guard runs in populate(), before any descriptor is built, and clears the partial result so callers never see a half-populated list.
  • Differential testing for arithmetic guards. Comparing against exact 128-bit arithmetic over millions of random inputs checks the boundary far more thoroughly than a handful of hand-picked cases.
  • Scope discipline. The overlapping covers() wrap already had an open PR, so I left it alone and said so rather than racing it.