Skip to content

Commit 55b5f3d

Browse files
superm1claude
authored andcommitted
[rocm-libraries] ROCm/rocm-libraries#9372 (commit 4488e33)
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>
1 parent e3f928d commit 55b5f3d

4 files changed

Lines changed: 211 additions & 2 deletions

File tree

test/hipstdpar/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ set(ROCPRIM_LOCATION ${ROCPRIM_INCLUDE_DIR})
103103
# Add tests
104104
add_hipstdpar_test("algorithms" "compile" OFF)
105105
add_hipstdpar_test("interpose" "compile" ON)
106+
add_hipstdpar_test("realloc" "run" ON)
106107

107108
# Restore global state
108109
set(CMAKE_CXX_STANDARD ${ROCTHRUST_CMAKE_CXX_STANDARD})

test/hipstdpar/test_realloc.cpp

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
// MIT License
2+
//
3+
// Copyright (c) 2026 Advanced Micro Devices, Inc. All rights reserved.
4+
//
5+
// Permission is hereby granted, free of charge, to any person obtaining a copy
6+
// of this software and associated documentation files (the "Software"), to deal
7+
// in the Software without restriction, including without limitation the rights
8+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
9+
// copies of the Software, and to permit persons to whom the Software is
10+
// furnished to do so, subject to the following conditions:
11+
//
12+
// The above copyright notice and this permission notice shall be included in all
13+
// copies or substantial portions of the Software.
14+
//
15+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
16+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
18+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
19+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
21+
// SOFTWARE.
22+
23+
// Regression test for __hipstdpar_realloc, which is what std::realloc resolves
24+
// to when translation units are compiled with --hipstdpar-interpose-alloc.
25+
//
26+
// The interposer used to copy n (the requested new size) bytes out of the old
27+
// block unconditionally. Growing an allocation therefore over-read past the end
28+
// of the old block, and realloc(nullptr, n) fed a NULL pointer straight to
29+
// memcpy. This test exercises the fixed contract:
30+
// * realloc(nullptr, n) behaves like malloc(n),
31+
// * realloc(p, 0) frees p and returns nullptr,
32+
// * growing preserves every byte of the old block (no over-read),
33+
// * shrinking preserves the retained prefix.
34+
//
35+
// Interposed allocations are backed by hipMallocManaged, so the storage is
36+
// host-accessible and the payload can be validated directly on the host.
37+
38+
#include <cstddef>
39+
#include <cstdint>
40+
#include <cstdlib>
41+
42+
namespace
43+
{
44+
// Deterministic, size-dependent fill so a stale/short copy is detectable.
45+
std::uint8_t pattern(std::size_t i)
46+
{
47+
return static_cast<std::uint8_t>((i * 31u + 7u) & 0xffu);
48+
}
49+
50+
void fill(std::uint8_t* p, std::size_t n)
51+
{
52+
for (std::size_t i = 0; i < n; ++i)
53+
{
54+
p[i] = pattern(i);
55+
}
56+
}
57+
58+
bool verify(const std::uint8_t* p, std::size_t n)
59+
{
60+
for (std::size_t i = 0; i < n; ++i)
61+
{
62+
if (p[i] != pattern(i))
63+
{
64+
return false;
65+
}
66+
}
67+
return true;
68+
}
69+
} // namespace
70+
71+
int main()
72+
{
73+
try
74+
{
75+
// realloc(nullptr, n) must behave like malloc(n).
76+
{
77+
auto p = static_cast<std::uint8_t*>(std::realloc(nullptr, 64));
78+
if (!p)
79+
{
80+
return EXIT_FAILURE;
81+
}
82+
fill(p, 64);
83+
if (!verify(p, 64))
84+
{
85+
return EXIT_FAILURE;
86+
}
87+
std::free(p);
88+
}
89+
90+
// realloc(p, 0) must free p and return nullptr.
91+
{
92+
auto p = std::malloc(64);
93+
if (!p)
94+
{
95+
return EXIT_FAILURE;
96+
}
97+
if (std::realloc(p, 0) != nullptr)
98+
{
99+
return EXIT_FAILURE;
100+
}
101+
}
102+
103+
// Growing must preserve the whole old block without over-reading past it.
104+
{
105+
constexpr std::size_t old_size = 32;
106+
constexpr std::size_t new_size = 8192;
107+
108+
auto p = static_cast<std::uint8_t*>(std::malloc(old_size));
109+
if (!p)
110+
{
111+
return EXIT_FAILURE;
112+
}
113+
fill(p, old_size);
114+
115+
auto q = static_cast<std::uint8_t*>(std::realloc(p, new_size));
116+
if (!q)
117+
{
118+
return EXIT_FAILURE;
119+
}
120+
if (!verify(q, old_size))
121+
{
122+
std::free(q);
123+
return EXIT_FAILURE;
124+
}
125+
std::free(q);
126+
}
127+
128+
// Shrinking must preserve the retained prefix.
129+
{
130+
constexpr std::size_t old_size = 8192;
131+
constexpr std::size_t new_size = 32;
132+
133+
auto p = static_cast<std::uint8_t*>(std::malloc(old_size));
134+
if (!p)
135+
{
136+
return EXIT_FAILURE;
137+
}
138+
fill(p, old_size);
139+
140+
auto q = static_cast<std::uint8_t*>(std::realloc(p, new_size));
141+
if (!q)
142+
{
143+
return EXIT_FAILURE;
144+
}
145+
if (!verify(q, new_size))
146+
{
147+
std::free(q);
148+
return EXIT_FAILURE;
149+
}
150+
std::free(q);
151+
}
152+
}
153+
catch (...)
154+
{
155+
return EXIT_FAILURE;
156+
}
157+
158+
return EXIT_SUCCESS;
159+
}

thrust/system/hip/hipstdpar/impl/interpose_allocations_v0.hpp

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,19 @@
4242
# if defined(__HIPSTDPAR_INTERPOSE_ALLOC__)
4343
# include <hip/hip_runtime.h>
4444

45+
# include <algorithm>
4546
# include <cstddef>
4647
# include <cstdint>
4748
# include <cstring>
4849
# include <memory_resource>
4950
# include <new>
5051
# include <stdexcept>
5152

53+
# if __has_include(<malloc.h>)
54+
# include <malloc.h>
55+
# define __HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__
56+
# endif
57+
5258
namespace hipstd
5359
{
5460
struct Header
@@ -132,7 +138,22 @@ extern "C" __attribute__((weak)) void __hipstdpar_hidden_free(void*);
132138

133139
extern "C" inline __attribute__((used)) void* __hipstdpar_realloc(void* p, std::size_t n)
134140
{
135-
auto q = std::memcpy(__hipstdpar_malloc(n), p, n);
141+
if (!p)
142+
{
143+
return __hipstdpar_malloc(n);
144+
}
145+
146+
if (n == 0)
147+
{
148+
__hipstdpar_free(p);
149+
return nullptr;
150+
}
151+
152+
auto q = __hipstdpar_malloc(n);
153+
if (!q)
154+
{
155+
return nullptr;
156+
}
136157

137158
auto h = static_cast<hipstd::Header*>(p) - 1;
138159

@@ -141,10 +162,18 @@ extern "C" inline __attribute__((used)) void* __hipstdpar_realloc(void* p, std::
141162

142163
if (!tmp.isManaged)
143164
{
165+
std::size_t old = n;
166+
# if defined(__HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__)
167+
old = malloc_usable_size(p);
168+
# endif
169+
std::memcpy(q, p, std::min(old, n));
144170
__hipstdpar_hidden_free(p);
145171
}
146172
else
147173
{
174+
const auto old = reinterpret_cast<std::uintptr_t>(h->alloc_ptr) + h->size
175+
- reinterpret_cast<std::uintptr_t>(p);
176+
std::memcpy(q, p, std::min<std::size_t>(old, n));
148177
hipstd::heap.deallocate(h->alloc_ptr, h->size, h->align);
149178
}
150179

thrust/system/hip/hipstdpar/impl/interpose_allocations_v1.hpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <sys/unistd.h>
3434
#endif
3535

36+
#include <algorithm>
3637
#include <climits>
3738
#include <cstddef>
3839
#include <cstdint>
@@ -42,6 +43,11 @@
4243
#include <stdexcept>
4344
#include <utility>
4445

46+
#if __has_include(<malloc.h>)
47+
#include <malloc.h>
48+
#define __HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__
49+
#endif
50+
4551
extern "C" {
4652
__attribute__((weak)) void __hipstdpar_hidden_free(void*);
4753
__attribute__((weak)) void* __hipstdpar_hidden_memalign(::std::size_t,
@@ -165,7 +171,21 @@ extern "C" {
165171
inline __attribute__((used)) void* __hipstdpar_realloc(void* p,
166172
std::size_t n)
167173
{
168-
auto q = std::memcpy(__hipstdpar_malloc(n), p, n);
174+
if (!p) return __hipstdpar_malloc(n);
175+
176+
if (n == 0) {
177+
__hipstdpar_free(p);
178+
return nullptr;
179+
}
180+
181+
auto q = __hipstdpar_malloc(n);
182+
if (!q) return nullptr;
183+
184+
std::size_t old = n;
185+
#if defined(__HIPSTDPAR_HAS_MALLOC_USABLE_SIZE__)
186+
old = malloc_usable_size(p);
187+
#endif
188+
std::memcpy(q, p, std::min(old, n));
169189
__hipstdpar_free(p);
170190

171191
return q;

0 commit comments

Comments
 (0)