From 4354192f862a5855d315ecf4d8589c671e14144f Mon Sep 17 00:00:00 2001 From: Mario Limonciello Date: Fri, 12 Jun 2026 13:58:02 -0500 Subject: [PATCH] [rocThrust] Fix over-read and NULL deref in __hipstdpar_realloc __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) --- .../rocthrust/test/hipstdpar/CMakeLists.txt | 1 + .../rocthrust/test/hipstdpar/test_realloc.cpp | 159 ++++++++++++++++++ .../impl/interpose_allocations_v0.hpp | 31 +++- .../impl/interpose_allocations_v1.hpp | 22 ++- 4 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 projects/rocthrust/test/hipstdpar/test_realloc.cpp diff --git a/projects/rocthrust/test/hipstdpar/CMakeLists.txt b/projects/rocthrust/test/hipstdpar/CMakeLists.txt index 6a68393a4686..2f7853a64573 100644 --- a/projects/rocthrust/test/hipstdpar/CMakeLists.txt +++ b/projects/rocthrust/test/hipstdpar/CMakeLists.txt @@ -104,6 +104,7 @@ set(ROCPRIM_LOCATION ${ROCPRIM_INCLUDE_DIR}) # Add tests add_hipstdpar_test("algorithms" "compile" OFF) add_hipstdpar_test("interpose" "compile" ON) +add_hipstdpar_test("realloc" "run" ON) # Restore global state set(CMAKE_CXX_STANDARD ${ROCTHRUST_CMAKE_CXX_STANDARD}) diff --git a/projects/rocthrust/test/hipstdpar/test_realloc.cpp b/projects/rocthrust/test/hipstdpar/test_realloc.cpp new file mode 100644 index 000000000000..534bdfb4978d --- /dev/null +++ b/projects/rocthrust/test/hipstdpar/test_realloc.cpp @@ -0,0 +1,159 @@ +// MIT License +// +// Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +// Regression test for __hipstdpar_realloc, which is what std::realloc resolves +// to when translation units are compiled with --hipstdpar-interpose-alloc. +// +// The interposer used to copy n (the requested new size) bytes out of the old +// block unconditionally. Growing an allocation therefore over-read past the end +// of the old block, and realloc(nullptr, n) fed a NULL pointer straight to +// memcpy. This test exercises the fixed contract: +// * realloc(nullptr, n) behaves like malloc(n), +// * realloc(p, 0) frees p and returns nullptr, +// * growing preserves every byte of the old block (no over-read), +// * shrinking preserves the retained prefix. +// +// Interposed allocations are backed by hipMallocManaged, so the storage is +// host-accessible and the payload can be validated directly on the host. + +#include +#include +#include + +namespace +{ +// Deterministic, size-dependent fill so a stale/short copy is detectable. +std::uint8_t pattern(std::size_t i) +{ + return static_cast((i * 31u + 7u) & 0xffu); +} + +void fill(std::uint8_t* p, std::size_t n) +{ + for (std::size_t i = 0; i < n; ++i) + { + p[i] = pattern(i); + } +} + +bool verify(const std::uint8_t* p, std::size_t n) +{ + for (std::size_t i = 0; i < n; ++i) + { + if (p[i] != pattern(i)) + { + return false; + } + } + return true; +} +} // namespace + +int main() +{ + try + { + // realloc(nullptr, n) must behave like malloc(n). + { + auto p = static_cast(std::realloc(nullptr, 64)); + if (!p) + { + return EXIT_FAILURE; + } + fill(p, 64); + if (!verify(p, 64)) + { + return EXIT_FAILURE; + } + std::free(p); + } + + // realloc(p, 0) must free p and return nullptr. + { + auto p = std::malloc(64); + if (!p) + { + return EXIT_FAILURE; + } + if (std::realloc(p, 0) != nullptr) + { + return EXIT_FAILURE; + } + } + + // Growing must preserve the whole old block without over-reading past it. + { + constexpr std::size_t old_size = 32; + constexpr std::size_t new_size = 8192; + + auto p = static_cast(std::malloc(old_size)); + if (!p) + { + return EXIT_FAILURE; + } + fill(p, old_size); + + auto q = static_cast(std::realloc(p, new_size)); + if (!q) + { + return EXIT_FAILURE; + } + if (!verify(q, old_size)) + { + std::free(q); + return EXIT_FAILURE; + } + std::free(q); + } + + // Shrinking must preserve the retained prefix. + { + constexpr std::size_t old_size = 8192; + constexpr std::size_t new_size = 32; + + auto p = static_cast(std::malloc(old_size)); + if (!p) + { + return EXIT_FAILURE; + } + fill(p, old_size); + + auto q = static_cast(std::realloc(p, new_size)); + if (!q) + { + return EXIT_FAILURE; + } + if (!verify(q, new_size)) + { + std::free(q); + return EXIT_FAILURE; + } + std::free(q); + } + } + catch (...) + { + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; +} diff --git a/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v0.hpp b/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v0.hpp index 5f33f2337ed8..8b978e7929cf 100644 --- a/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v0.hpp +++ b/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v0.hpp @@ -42,6 +42,7 @@ # if defined(__HIPSTDPAR_INTERPOSE_ALLOC__) # include +# include # include # include # include @@ -49,6 +50,11 @@ # include # include +# if __has_include() +# include +# define __HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__ +# endif + namespace hipstd { struct Header @@ -132,7 +138,22 @@ extern "C" __attribute__((weak)) void __hipstdpar_hidden_free(void*); extern "C" inline __attribute__((used)) void* __hipstdpar_realloc(void* p, std::size_t n) { - auto q = std::memcpy(__hipstdpar_malloc(n), p, n); + if (!p) + { + return __hipstdpar_malloc(n); + } + + if (n == 0) + { + __hipstdpar_free(p); + return nullptr; + } + + auto q = __hipstdpar_malloc(n); + if (!q) + { + return nullptr; + } auto h = static_cast(p) - 1; @@ -141,10 +162,18 @@ extern "C" inline __attribute__((used)) void* __hipstdpar_realloc(void* p, std:: if (!tmp.isManaged) { + std::size_t old = n; +# if defined(__HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__) + old = malloc_usable_size(p); +# endif + std::memcpy(q, p, std::min(old, n)); __hipstdpar_hidden_free(p); } else { + const auto old = reinterpret_cast(h->alloc_ptr) + h->size + - reinterpret_cast(p); + std::memcpy(q, p, std::min(old, n)); hipstd::heap.deallocate(h->alloc_ptr, h->size, h->align); } diff --git a/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v1.hpp b/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v1.hpp index 0371247e0cf1..ab882fe4e1eb 100644 --- a/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v1.hpp +++ b/projects/rocthrust/thrust/system/hip/hipstdpar/impl/interpose_allocations_v1.hpp @@ -33,6 +33,7 @@ #include #endif +#include #include #include #include @@ -42,6 +43,11 @@ #include #include +#if __has_include() + #include + #define __HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__ +#endif + extern "C" { __attribute__((weak)) void __hipstdpar_hidden_free(void*); __attribute__((weak)) void* __hipstdpar_hidden_memalign(::std::size_t, @@ -165,7 +171,21 @@ extern "C" { inline __attribute__((used)) void* __hipstdpar_realloc(void* p, std::size_t n) { - auto q = std::memcpy(__hipstdpar_malloc(n), p, n); + if (!p) return __hipstdpar_malloc(n); + + if (n == 0) { + __hipstdpar_free(p); + return nullptr; + } + + auto q = __hipstdpar_malloc(n); + if (!q) return nullptr; + + std::size_t old = n; + #if defined(__HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__) + old = malloc_usable_size(p); + #endif + std::memcpy(q, p, std::min(old, n)); __hipstdpar_free(p); return q;