Fail when a requested RDMA device is not found - #4180
Open
erwinzhang7 wants to merge 1 commit into
Open
Conversation
create_connections() skipped names that matched no enumerated device without reporting anything, so it returned fewer connections than it was given names. Its callers size themselves from device_names, so the extra indices read past the end of the vector and the garbage was passed to ibv_reg_mr as a protection domain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3777.
create_connections()walksdevice_namesand, for each non-empty name, searches theenumerated verbs devices. If the name matches nothing the inner loop simply falls out and the
next name is processed — nothing is appended and nothing is reported. It throws when
open_devicefails, but not when the device is absent entirely.The result is a vector shorter than
device_names.MeshGroupandRingGroupsizethemselves from
device_names(size_(device_names.size())), so every index fromconnections.size()upward reads past the end, and the garbage is passed toibv_reg_mras aprotection domain.
This is why it is not a missing null check: the read is out of bounds before there is a
pointer to check. With
-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUGthe same runaborts at
vector.h:406,libc++ Hardening assertion __n < size() failed: vector[] index out of bounds.Reproduction
No RDMA hardware and no second node are needed — the absence of a verbs device is what
triggers it. On macOS 26.6 with
rdma_ctl statusreportingdisabled,librdma.dylibstillloads and every
ibv_*symbol resolves, sojaccl::is_available()returnstruewhileibv_get_device_listreports zero devices.std::vector<std::string> names = {"", "rdma_bogus0"}; jaccl::AllGatherFn agf = [](const char* s, char* d, size_t n) { std::memcpy(d, s, n); }; jaccl::MeshGroup mesh(0, names, jaccl::SideChannel(0, names.size(), agf));Single process, stub all-gather, no coordinator. The stub is never called: the constructor
dies earlier, inside
MeshGroup::allocate_buffers().After this change the same program throws instead:
Fixing it in
create_connectionsrather than at the call sites means the invariant thecallers already rely on — that the returned vector is as long as
device_names— is restoredat its source, so no bounds or null guards are needed downstream.
Checked
Valid inputs are unaffected: an empty list returns 0, and lists of empty self-slots return one
connection each (1 → 1, 3 → 3). Absent names now throw whether they appear first or last.
Notes
The existing
open_deviceandMESH_MAX_PEERSfailures also throw without callingfree_device_list, and I matched that rather than changing the surrounding behaviour. Happyto add the cleanup on all three paths if you would prefer.
I did not add an automated test: there is no jaccl test target today —
testslinks onlymlxanddoctest, and jaccl is aPRIVATElink insidemlx, socreate_connectionsisnot reachable from it — and jaccl itself only builds on macOS SDK ≥ 26.2. Adding that
scaffolding seemed larger than the fix. Glad to add a guarded test if you would like one.
Out of scope: @cdvankammen's report on this issue of the same crash with
rdma_en2present on both hosts is not addressed here and I could not verify it — I have no
RDMA-enumerating hardware. This PR only covers the fail-fast path for an absent device.