Skip to content

Commit 33978eb

Browse files
committed
[L0] Refcnt Parent Buffer on Sub Buffer Create and die on use of buffer after free
- Refcnt the Parent Buffer on Sub Buffer Creation to avoid free until after all sub buffers are released. - Avoid use after free of buffers by tracking if a buffer has already been called with free() and verify that the parent of a sub buffer has not already been released before attempting to get ze handles from the parent. - In either use after free case, die aborts before the memory is accessed. Signed-off-by: Neil R. Spruit <[email protected]>
1 parent 3217838 commit 33978eb

File tree

2 files changed

+30
-12
lines changed

2 files changed

+30
-12
lines changed

source/adapters/level_zero/memory.cpp

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,19 +2045,30 @@ ur_result_t _ur_buffer::getZeHandle(char *&ZeHandle, access_mode_t AccessMode,
20452045

20462046
auto &Allocation = Allocations[Device];
20472047

2048+
if (this->isFreed) {
2049+
die("getZeHandle() buffer already released, no valid handles.");
2050+
}
2051+
20482052
// Sub-buffers don't maintain own allocations but rely on parent buffer.
20492053
if (SubBuffer) {
2050-
UR_CALL(SubBuffer->Parent->getZeHandle(ZeHandle, AccessMode, Device,
2051-
phWaitEvents, numWaitEvents));
2052-
ZeHandle += SubBuffer->Origin;
2053-
// Still store the allocation info in the PI sub-buffer for
2054-
// getZeHandlePtr to work. At least zeKernelSetArgumentValue needs to
2055-
// be given a pointer to the allocation handle rather than its value.
2056-
//
2057-
Allocation.ZeHandle = ZeHandle;
2058-
Allocation.ReleaseAction = allocation_t::keep;
2059-
LastDeviceWithValidAllocation = Device;
2060-
return UR_RESULT_SUCCESS;
2054+
// Verify that the Parent Buffer is still valid or if it has been freed.
2055+
if (SubBuffer->Parent && !SubBuffer->Parent->isFreed) {
2056+
UR_CALL(SubBuffer->Parent->getZeHandle(ZeHandle, AccessMode, Device,
2057+
phWaitEvents, numWaitEvents));
2058+
ZeHandle += SubBuffer->Origin;
2059+
// Still store the allocation info in the PI sub-buffer for
2060+
// getZeHandlePtr to work. At least zeKernelSetArgumentValue needs to
2061+
// be given a pointer to the allocation handle rather than its value.
2062+
//
2063+
Allocation.ZeHandle = ZeHandle;
2064+
Allocation.ReleaseAction = allocation_t::keep;
2065+
LastDeviceWithValidAllocation = Device;
2066+
return UR_RESULT_SUCCESS;
2067+
} else {
2068+
// Return an error if the parent buffer is already gone.
2069+
die("getZeHandle() SubBuffer's parent already released, no valid "
2070+
"handles.");
2071+
}
20612072
}
20622073

20632074
// First handle case where the buffer is represented by only
@@ -2320,6 +2331,7 @@ ur_result_t _ur_buffer::free() {
23202331
die("_ur_buffer::free(): Unhandled release action");
23212332
}
23222333
ZeHandle = nullptr; // don't leave hanging pointers
2334+
this->isFreed = true;
23232335
}
23242336
return UR_RESULT_SUCCESS;
23252337
}

source/adapters/level_zero/memory.hpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ struct _ur_buffer final : ur_mem_handle_t_ {
108108
// Sub-buffer constructor
109109
_ur_buffer(_ur_buffer *Parent, size_t Origin, size_t Size)
110110
: ur_mem_handle_t_(Parent->UrContext),
111-
Size(Size), SubBuffer{{Parent, Origin}} {}
111+
Size(Size), SubBuffer{{Parent, Origin}} {
112+
// Retain the Parent Buffer due to the Creation of the SubBuffer.
113+
Parent->RefCount.increment();
114+
}
112115

113116
// Interop-buffer constructor
114117
_ur_buffer(ur_context_handle_t Context, size_t Size,
@@ -136,6 +139,9 @@ struct _ur_buffer final : ur_mem_handle_t_ {
136139
// Frees all allocations made for the buffer.
137140
ur_result_t free();
138141

142+
// Tracks if this buffer is freed already or should be considered valid.
143+
bool isFreed{false};
144+
139145
// Information about a single allocation representing this buffer.
140146
struct allocation_t {
141147
// Level Zero memory handle is really just a naked pointer.

0 commit comments

Comments
 (0)