Skip to content

Commit aa088da

Browse files
User pointer read-only memory fix
- do not store fragment in map until hostPointerValidation is done - set pointers to nullptr after delete in cleanOsHandles Change-Id: I0bf99c3215c4b91ce059bb4e94716671c49f1946
1 parent 125d948 commit aa088da

File tree

7 files changed

+211
-80
lines changed

7 files changed

+211
-80
lines changed

runtime/memory_manager/host_ptr_defines.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, Intel Corporation
2+
* Copyright (c) 2017 - 2018, Intel Corporation
33
*
44
* Permission is hereby granted, free of charge, to any person obtaining a
55
* copy of this software and associated documentation files (the "Software"),
@@ -71,7 +71,7 @@ struct AllocationRequirements {
7171
};
7272

7373
struct FragmentStorage {
74-
void *fragmentCpuPointer = nullptr;
74+
const void *fragmentCpuPointer = nullptr;
7575
size_t fragmentSize = 0;
7676
int refCount = 0;
7777
OsHandle *osInternalStorage = nullptr;

runtime/memory_manager/host_ptr_manager.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, Intel Corporation
2+
* Copyright (c) 2017 - 2018, Intel Corporation
33
*
44
* Permission is hereby granted, free of charge, to any person obtaining a
55
* copy of this software and associated documentation files (the "Software"),
@@ -26,7 +26,7 @@
2626

2727
using namespace OCLRT;
2828

29-
std::map<void *, FragmentStorage>::iterator OCLRT::HostPtrManager::findElement(void *ptr) {
29+
std::map<const void *, FragmentStorage>::iterator OCLRT::HostPtrManager::findElement(const void *ptr) {
3030
auto nextElement = partialAllocations.lower_bound(ptr);
3131
auto element = nextElement;
3232
if (element != partialAllocations.end()) {
@@ -155,7 +155,7 @@ void OCLRT::HostPtrManager::storeFragment(FragmentStorage &fragment) {
155155
element->second.refCount++;
156156
} else {
157157
fragment.refCount++;
158-
partialAllocations.insert(std::pair<void *, FragmentStorage>(fragment.fragmentCpuPointer, fragment));
158+
partialAllocations.insert(std::pair<const void *, FragmentStorage>(fragment.fragmentCpuPointer, fragment));
159159
}
160160
}
161161

@@ -171,12 +171,12 @@ void OCLRT::HostPtrManager::storeFragment(AllocationStorageData &storageData) {
171171
void OCLRT::HostPtrManager::releaseHandleStorage(OsHandleStorage &fragments) {
172172
for (int i = 0; i < max_fragments_count; i++) {
173173
if (fragments.fragmentStorageData[i].fragmentSize || fragments.fragmentStorageData[i].cpuPtr) {
174-
fragments.fragmentStorageData[i].freeTheFragment = releaseHostPtr(const_cast<void *>(fragments.fragmentStorageData[i].cpuPtr));
174+
fragments.fragmentStorageData[i].freeTheFragment = releaseHostPtr(fragments.fragmentStorageData[i].cpuPtr);
175175
}
176176
}
177177
}
178178

179-
bool OCLRT::HostPtrManager::releaseHostPtr(void *ptr) {
179+
bool OCLRT::HostPtrManager::releaseHostPtr(const void *ptr) {
180180
std::lock_guard<std::mutex> lock(allocationsMutex);
181181
bool fragmentReadyToBeReleased = false;
182182

@@ -193,7 +193,7 @@ bool OCLRT::HostPtrManager::releaseHostPtr(void *ptr) {
193193
return fragmentReadyToBeReleased;
194194
}
195195

196-
FragmentStorage *OCLRT::HostPtrManager::getFragment(void *inputPtr) {
196+
FragmentStorage *OCLRT::HostPtrManager::getFragment(const void *inputPtr) {
197197
std::lock_guard<std::mutex> lock(allocationsMutex);
198198
auto element = findElement(inputPtr);
199199
if (element != partialAllocations.end()) {

runtime/memory_manager/host_ptr_manager.h

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, Intel Corporation
2+
* Copyright (c) 2017 - 2018, Intel Corporation
33
*
44
* Permission is hereby granted, free of charge, to any person obtaining a
55
* copy of this software and associated documentation files (the "Software"),
@@ -28,7 +28,7 @@
2828

2929
namespace OCLRT {
3030

31-
typedef std::map<void *, FragmentStorage> HostPtrFragmentsContainer;
31+
typedef std::map<const void *, FragmentStorage> HostPtrFragmentsContainer;
3232

3333
class HostPtrManager {
3434
public:
@@ -38,14 +38,14 @@ class HostPtrManager {
3838
void storeFragment(AllocationStorageData &storageData);
3939

4040
void releaseHandleStorage(OsHandleStorage &fragments);
41-
bool releaseHostPtr(void *ptr);
41+
bool releaseHostPtr(const void *ptr);
4242

43-
FragmentStorage *getFragment(void *inputPtr);
43+
FragmentStorage *getFragment(const void *inputPtr);
4444
size_t getFragmentCount() { return partialAllocations.size(); }
4545
FragmentStorage *getFragmentAndCheckForOverlaps(const void *inputPtr, size_t size, OverlapStatus &overlappingStatus);
4646

4747
private:
48-
std::map<void *, FragmentStorage>::iterator findElement(void *ptr);
48+
std::map<const void *, FragmentStorage>::iterator findElement(const void *ptr);
4949

5050
HostPtrFragmentsContainer partialAllocations;
5151
std::mutex allocationsMutex;

runtime/os_interface/linux/drm_memory_manager.cpp

+7-3
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ uint64_t DrmMemoryManager::getInternalHeapBaseAddress() {
471471

472472
MemoryManager::AllocationStatus DrmMemoryManager::populateOsHandles(OsHandleStorage &handleStorage) {
473473
BufferObject *allocatedBos[max_fragments_count];
474-
size_t numberOfBosAllocated = 0;
474+
uint32_t numberOfBosAllocated = 0;
475475
uint32_t indexesOfAllocatedBos[max_fragments_count];
476476

477477
for (unsigned int i = 0; i < max_fragments_count; i++) {
@@ -492,8 +492,6 @@ MemoryManager::AllocationStatus DrmMemoryManager::populateOsHandles(OsHandleStor
492492
allocatedBos[numberOfBosAllocated] = handleStorage.fragmentStorageData[i].osHandleStorage->bo;
493493
indexesOfAllocatedBos[numberOfBosAllocated] = i;
494494
numberOfBosAllocated++;
495-
496-
hostPtrManager.storeFragment(handleStorage.fragmentStorageData[i]);
497495
}
498496
}
499497

@@ -510,8 +508,12 @@ MemoryManager::AllocationStatus DrmMemoryManager::populateOsHandles(OsHandleStor
510508
}
511509
}
512510

511+
for (uint32_t i = 0; i < numberOfBosAllocated; i++) {
512+
hostPtrManager.storeFragment(handleStorage.fragmentStorageData[indexesOfAllocatedBos[i]]);
513+
}
513514
return AllocationStatus::Success;
514515
}
516+
515517
void DrmMemoryManager::cleanOsHandles(OsHandleStorage &handleStorage) {
516518
for (unsigned int i = 0; i < max_fragments_count; i++) {
517519
if (handleStorage.fragmentStorageData[i].freeTheFragment) {
@@ -523,7 +525,9 @@ void DrmMemoryManager::cleanOsHandles(OsHandleStorage &handleStorage) {
523525
((void)(refCount));
524526
}
525527
delete handleStorage.fragmentStorageData[i].osHandleStorage;
528+
handleStorage.fragmentStorageData[i].osHandleStorage = nullptr;
526529
delete handleStorage.fragmentStorageData[i].residency;
530+
handleStorage.fragmentStorageData[i].residency = nullptr;
527531
}
528532
}
529533
}

runtime/os_interface/windows/wddm_memory_manager.cpp

+10-1
Original file line numberDiff line numberDiff line change
@@ -337,21 +337,30 @@ bool WddmMemoryManager::validateAllocation(WddmAllocation *alloc) {
337337
}
338338

339339
MemoryManager::AllocationStatus WddmMemoryManager::populateOsHandles(OsHandleStorage &handleStorage) {
340+
uint32_t allocatedFragmentIndexes[max_fragments_count];
341+
uint32_t allocatedFragmentsCounter = 0;
342+
340343
for (unsigned int i = 0; i < max_fragments_count; i++) {
341344
// If no fragment is present it means it already exists.
342345
if (!handleStorage.fragmentStorageData[i].osHandleStorage && handleStorage.fragmentStorageData[i].cpuPtr) {
343346
handleStorage.fragmentStorageData[i].osHandleStorage = new OsHandle();
344347
handleStorage.fragmentStorageData[i].residency = new ResidencyData();
345348

346349
handleStorage.fragmentStorageData[i].osHandleStorage->gmm = Gmm::create(handleStorage.fragmentStorageData[i].cpuPtr, handleStorage.fragmentStorageData[i].fragmentSize, false);
347-
hostPtrManager.storeFragment(handleStorage.fragmentStorageData[i]);
350+
allocatedFragmentIndexes[allocatedFragmentsCounter] = i;
351+
allocatedFragmentsCounter++;
348352
}
349353
}
350354
NTSTATUS result = wddm->createAllocationsAndMapGpuVa(handleStorage);
351355

352356
if (result == STATUS_GRAPHICS_NO_VIDEO_MEMORY) {
353357
return AllocationStatus::InvalidHostPointer;
354358
}
359+
360+
for (uint32_t i = 0; i < allocatedFragmentsCounter; i++) {
361+
hostPtrManager.storeFragment(handleStorage.fragmentStorageData[allocatedFragmentIndexes[i]]);
362+
}
363+
355364
return AllocationStatus::Success;
356365
}
357366

0 commit comments

Comments
 (0)