Skip to content

fix: Fix over-read and NULL deref in __hipstdpar_realloc#9372

Merged
superm1 merged 1 commit into
developfrom
users/superm1/SWSPLAT-24298
Jul 15, 2026
Merged

fix: Fix over-read and NULL deref in __hipstdpar_realloc#9372
superm1 merged 1 commit into
developfrom
users/superm1/SWSPLAT-24298

Conversation

@superm1

@superm1 superm1 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

__hipstdpar_realloc copied n (the new size) bytes from the old pointer, over-reading past the end of the old block when growing an allocation. It also dereferenced the interposer Header for non-interposed pointers and passed a NULL pointer straight to memcpy on realloc(NULL, n).

Clamp the copy to min(old_size, n), handle realloc(NULL, n) as malloc and realloc(p, 0) as free, and only decode the Header for managed allocations. The old size comes from the stored Header (v0) or malloc_usable_size (v1 and foreign pointers, when available).

JIRA ID: SWSPLAT-24298

@therock-pr-bot

therock-pr-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

@therock-pr-bot

therock-pr-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

❌ Your project status has failed because the head coverage (76.84%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #9372      +/-   ##
===========================================
- Coverage    64.60%   64.60%   -0.00%     
===========================================
  Files         2686     2686              
  Lines       423252   423251       -1     
  Branches     62931    62932       +1     
===========================================
- Hits        273421   273416       -5     
- Misses      129157   129160       +3     
- Partials     20674    20675       +1     
Flag Coverage Δ *Carryforward flag
TensileLite 34.70% <ø> (ø) Carriedforward from c4883c1
hipBLAS 90.81% <ø> (ø) Carriedforward from c4883c1
hipBLASLt 34.69% <ø> (ø) Carriedforward from c4883c1
hipCUB 82.68% <ø> (ø) Carriedforward from c4883c1
hipDNN 86.03% <ø> (ø) Carriedforward from c4883c1
hipFFT 50.88% <ø> (ø) Carriedforward from c4883c1
hipRAND 76.12% <ø> (ø) Carriedforward from c4883c1
hipSOLVER 69.18% <ø> (ø) Carriedforward from c4883c1
hipSPARSE 86.10% <ø> (ø) Carriedforward from c4883c1
rocBLAS 47.92% <ø> (ø) Carriedforward from c4883c1
rocFFT 46.82% <ø> (ø) Carriedforward from c4883c1
rocRAND 57.03% <ø> (ø) Carriedforward from c4883c1
rocSOLVER 76.84% <ø> (ø) Carriedforward from c4883c1
rocSPARSE 72.37% <ø> (ø) Carriedforward from c4883c1
rocThrust 91.35% <ø> (-0.01%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.
see 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@superm1 superm1 changed the title fix(rocThrust): Fix over-read and NULL deref in __hipstdpar_realloc fix: Fix over-read and NULL deref in __hipstdpar_realloc Jul 14, 2026
Comment thread projects/rocthrust/test/hipstdpar/test_realloc.cpp Outdated

@amd-jmahovsky amd-jmahovsky left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the core fix looks OK but I have a question about the test.

Comment thread projects/rocthrust/test/hipstdpar/test_realloc.cpp
__hipstdpar_realloc copied n (the new size) bytes from the old pointer,
over-reading past the end of the old block when growing an allocation.
It also dereferenced the interposer Header for non-interposed pointers
and passed a NULL pointer straight to memcpy on realloc(NULL, n).

Clamp the copy to min(old_size, n), handle realloc(NULL, n) as malloc
and realloc(p, 0) as free, and only decode the Header for managed
allocations. The old size comes from the stored Header (v0) or
malloc_usable_size (v1 and foreign pointers, when available).

Fixes: SWSPLAT-24298

Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
@superm1 superm1 force-pushed the users/superm1/SWSPLAT-24298 branch from 24558e4 to 4354192 Compare July 14, 2026 19:36
@superm1 superm1 requested a review from NguyenNhuDi July 14, 2026 19:37
@superm1 superm1 merged commit 4488e33 into develop Jul 15, 2026
31 checks passed
@superm1 superm1 deleted the users/superm1/SWSPLAT-24298 branch July 15, 2026 14:02
assistant-librarian Bot pushed a commit to ROCm/rocThrust that referenced this pull request Jul 15, 2026
fix: Fix over-read and NULL deref in __hipstdpar_realloc
 (#9372)

__hipstdpar_realloc copied n (the new size) bytes from the old pointer,
over-reading past the end of the old block when growing an allocation.
It also dereferenced the interposer Header for non-interposed pointers
and passed a NULL pointer straight to memcpy on realloc(NULL, n).

Clamp the copy to min(old_size, n), handle realloc(NULL, n) as malloc
and realloc(p, 0) as free, and only decode the Header for managed
allocations. The old size comes from the stored Header (v0) or
malloc_usable_size (v1 and foreign pointers, when available).

JIRA ID: SWSPLAT-24298

Co-authored-by: Claude Opus 4 (1M context) <noreply@anthropic.com>
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.

4 participants