Skip to content

Commit

Permalink
Limit how much we allow D3D12 addresses to go out of bounds
Browse files Browse the repository at this point in the history
  • Loading branch information
baldurk committed Nov 6, 2023
1 parent dcbbcee commit b711a05
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 18 deletions.
2 changes: 1 addition & 1 deletion renderdoc/driver/d3d12/d3d12_debug.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1796,7 +1796,7 @@ void D3D12DebugManager::PrepareExecuteIndirectPatching(const GPUAddressRangeTrac
{
buffermapping b = {};
b.origBase = addr.start;
b.origEnd = addr.end;
b.origEnd = addr.realEnd;
b.newBase =
m_pDevice->GetResourceManager()->GetLiveAs<ID3D12Resource>(addr.id)->GetGPUVirtualAddress();
buffers.push_back(b);
Expand Down
4 changes: 2 additions & 2 deletions renderdoc/driver/d3d12/d3d12_device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1677,7 +1677,7 @@ bool WrappedID3D12Device::Serialise_WrapSwapchainBuffer(SerialiserType &ser, IDX
}
else
{
WrappedID3D12Resource *wrapped = new WrappedID3D12Resource(fakeBB, this);
WrappedID3D12Resource *wrapped = new WrappedID3D12Resource(fakeBB, NULL, 0, this);
fakeBB = wrapped;

fakeBB->SetName(L"Swap Chain Buffer");
Expand Down Expand Up @@ -1719,7 +1719,7 @@ IUnknown *WrappedID3D12Device::WrapSwapchainBuffer(IDXGISwapper *swapper, DXGI_F
}
else
{
pRes = new WrappedID3D12Resource((ID3D12Resource *)realSurface, this);
pRes = new WrappedID3D12Resource((ID3D12Resource *)realSurface, NULL, 0, this);

ResourceId id = GetResID(pRes);

Expand Down
21 changes: 16 additions & 5 deletions renderdoc/driver/d3d12/d3d12_device_rescreate_wrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,20 @@ bool WrappedID3D12Device::Serialise_CreateResource(
{
GPUAddressRange range;
range.start = gpuAddress;
range.end = gpuAddress + desc.Width;
range.realEnd = gpuAddress + desc.Width;

// if this is placed, the OOB end is all the way to the end of the heap, from where we're
// placed, allowing accesses past the buffer but still in bounds of the heap.
if(pHeap)
{
const UINT64 heapSize = pHeap->GetDesc().SizeInBytes;
range.oobEnd = gpuAddress + (heapSize - HeapOffset);
}
else
{
range.oobEnd = range.realEnd;
}

range.id = pResource;

m_OrigGPUAddresses.AddTo(range);
Expand Down Expand Up @@ -232,7 +245,7 @@ bool WrappedID3D12Device::Serialise_CreateResource(
SetObjName(ret, StringFormat::Fmt("%s Resource %s %s", ResourceTypeName,
ToStr(desc.Dimension).c_str(), ToStr(pResource).c_str()));

ret = new WrappedID3D12Resource(ret, this);
ret = new WrappedID3D12Resource(ret, pHeap, HeapOffset, this);

switch(chunkType)
{
Expand Down Expand Up @@ -422,9 +435,7 @@ HRESULT WrappedID3D12Device::CreateResource(

UINT NumSubresources = GetNumSubresources(m_pDevice, &desc);

WrappedID3D12Resource *wrapped = new WrappedID3D12Resource(realRes, this);

wrapped->SetHeap(pHeap);
WrappedID3D12Resource *wrapped = new WrappedID3D12Resource(realRes, pHeap, HeapOffset, this);

if(IsCaptureMode(m_State))
{
Expand Down
7 changes: 6 additions & 1 deletion renderdoc/driver/d3d12/d3d12_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ void GPUAddressRangeTracker::GetResIDFromAddr(D3D12_GPU_VIRTUAL_ADDRESS addr, Re
range = *it;
}

if(addr < range.start || addr >= range.end)
if(addr < range.start || addr >= range.realEnd)
return;

id = range.id;
Expand Down Expand Up @@ -1073,6 +1073,11 @@ void GPUAddressRangeTracker::GetResIDFromAddrAllowOutOfBounds(D3D12_GPU_VIRTUAL_
if(addr < range.start)
return;

// still enforce the OOB end on ranges - which is the remaining range in the backing store.
// Otherwise we could end up passing through invalid addresses stored in stale descriptors
if(addr >= range.oobEnd)
return;

id = range.id;
offs = addr - range.start;
}
2 changes: 1 addition & 1 deletion renderdoc/driver/d3d12/d3d12_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ class WrappedID3D12Resource;

struct GPUAddressRange
{
D3D12_GPU_VIRTUAL_ADDRESS start, end;
D3D12_GPU_VIRTUAL_ADDRESS start, realEnd, oobEnd;
ResourceId id;

bool operator<(const D3D12_GPU_VIRTUAL_ADDRESS &o) const
Expand Down
2 changes: 1 addition & 1 deletion renderdoc/driver/d3d12/d3d12_resources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ WrappedID3D12Resource::~WrappedID3D12Resource()
{
GPUAddressRange range;
range.start = m_pReal->GetGPUVirtualAddress();
range.end = m_pReal->GetGPUVirtualAddress() + m_pReal->GetDesc().Width;
// realEnd and oobEnd are not used for removing, just start + id
range.id = GetResourceID();

m_Addresses.RemoveFrom(range);
Expand Down
26 changes: 19 additions & 7 deletions renderdoc/driver/d3d12/d3d12_resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -953,11 +953,6 @@ class WrappedID3D12Resource
return this->GetResourceID();
}

void SetHeap(ID3D12Heap *heap)
{
m_Heap = (WrappedID3D12Heap *)heap;
SAFE_ADDREF(m_Heap);
}
static void RefBuffers(D3D12ResourceManager *rm);
static void GetMappableIDs(D3D12ResourceManager *rm, const std::unordered_set<ResourceId> &refdIDs,
std::unordered_set<ResourceId> &mappableIDs);
Expand Down Expand Up @@ -991,20 +986,37 @@ class WrappedID3D12Resource
TypeEnum = Resource_Resource,
};

WrappedID3D12Resource(ID3D12Resource *real, WrappedID3D12Device *device)
WrappedID3D12Resource(ID3D12Resource *real, ID3D12Heap *heap, UINT64 HeapOffset,
WrappedID3D12Device *device)
: WrappedDeviceChild12(real, device)
{
if(IsReplayMode(device->GetState()))
device->AddReplayResource(GetResourceID(), this);

m_Heap = (WrappedID3D12Heap *)heap;
SAFE_ADDREF(m_Heap);

// assuming only valid for buffers
if(m_pReal->GetDesc().Dimension == D3D12_RESOURCE_DIMENSION_BUFFER)
{
D3D12_GPU_VIRTUAL_ADDRESS addr = m_pReal->GetGPUVirtualAddress();

GPUAddressRange range;
range.start = addr;
range.end = addr + m_pReal->GetDesc().Width;
range.realEnd = addr + m_pReal->GetDesc().Width;

// if this is placed, the OOB end is all the way to the end of the heap, from where we're
// placed, allowing accesses past the buffer but still in bounds of the heap.
if(heap)
{
const UINT64 heapSize = heap->GetDesc().SizeInBytes;
range.oobEnd = addr + (heapSize - HeapOffset);
}
else
{
range.oobEnd = range.realEnd;
}

range.id = GetResourceID();

m_Addresses.AddTo(range);
Expand Down

0 comments on commit b711a05

Please sign in to comment.