Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net_ib: fix out of bounds read in ncclIbGdrSupport on non-RDMA kernel #1470

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

LunNova
Copy link
Contributor

@LunNova LunNova commented Dec 18, 2024

Fixes #1469

Details

Do not mention proprietary info or link to internal work items in this PR.

Work item: #1469

What were the changes?
Fixed out of bounds read and subsequent crash if none of the entries in memory_peers_path exist.

Why were the changes made?
Crashing is bad.

How was the outcome achieved?
Only crashes/demonstrates UB on a system that is missing all entries in this list.

Additional Documentation:
What else should the reviewer know?

Approval Checklist

Do not approve until these items are satisfied.

  • Verify the CHANGELOG has been updated, if
    • there are any NCCL API version changes,
    • any changes impact library users, and/or
    • any changes impact any other ROCm library.

@wenkaidu
Copy link
Collaborator

wenkaidu commented Jan 8, 2025

CI failure on 4gfx908 was in docker stage.
CI failure on 16gfx90a was known issue with FP8:
[ INFO ] SP 16 ranks AllReduce (OP,GM,NL) ncclFp8E4M3 sum [ 393216 elements]
../../../test/common/TestBed.cpp:396: Failure
Expected equality of these values:
isCorrect
Which is: false
true
Output does not match expected

Thus merging the PR.

@wenkaidu wenkaidu merged commit b24580e into ROCm:develop Jan 8, 2025
19 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue]: [UB] memory_peers_paths is missing null terminator causing out of bound reads
2 participants