From 0d0b68266cf17df1e37f828ce4c2fcc2b365bd83 Mon Sep 17 00:00:00 2001
From: Matthew Poremba <matthew.poremba@amd.com>
Date: Fri, 26 Jul 2024 10:38:27 -0700
Subject: [PATCH] dev-amdgpu: Fix bad free in SDMA

The SDMA engine copies data in chunks. It currently uses the pointer
returned from new[] and manipulates it using pointer arithmetic. This
modified pointer is then passed to the completion function which deletes
the pointer. Since it is not the original pointer allocated by new[]
this triggers issues in ASAN.

Change-Id: I03ccf026633285e75005509445c62fcbda8eb978
---
 src/dev/amdgpu/sdma_engine.cc | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/dev/amdgpu/sdma_engine.cc b/src/dev/amdgpu/sdma_engine.cc
index e1e123df48..582f128f82 100644
--- a/src/dev/amdgpu/sdma_engine.cc
+++ b/src/dev/amdgpu/sdma_engine.cc
@@ -653,7 +653,7 @@ SDMAEngine::writeDone(SDMAQueue *q, sdmaWrite *pkt, uint32_t *dmaBuffer)
 {
     DPRINTF(SDMAEngine, "Write packet completed to %p, %d dwords\n",
             pkt->dest, pkt->count);
-    delete []dmaBuffer;
+    delete [] dmaBuffer;
     delete pkt;
     decodeNext(q);
 }
@@ -686,6 +686,7 @@ SDMAEngine::copy(SDMAQueue *q, sdmaCopy *pkt)
         // Copy the minimum page size at a time in case the physical addresses
         // are not contiguous.
         ChunkGenerator gen(pkt->source, pkt->count, AMDGPU_MMHUB_PAGE_SIZE);
+        uint8_t *buffer_ptr = dmaBuffer;
         for (; !gen.done(); gen.next()) {
             Addr chunk_addr = getDeviceAddress(gen.addr());
             assert(chunk_addr);
@@ -693,10 +694,10 @@ SDMAEngine::copy(SDMAQueue *q, sdmaCopy *pkt)
             DPRINTF(SDMAEngine, "Copying chunk of %d bytes from %#lx (%#lx)\n",
                     gen.size(), gen.addr(), chunk_addr);
 
-            gpuDevice->getMemMgr()->readRequest(chunk_addr, dmaBuffer,
+            gpuDevice->getMemMgr()->readRequest(chunk_addr, buffer_ptr,
                                                 gen.size(), 0,
                                                 gen.last() ? cb : nullptr);
-            dmaBuffer += gen.size();
+            buffer_ptr += gen.size();
         }
     } else {
         auto cb = new DmaVirtCallback<uint64_t>(
@@ -731,6 +732,7 @@ SDMAEngine::copyReadData(SDMAQueue *q, sdmaCopy *pkt, uint8_t *dmaBuffer)
         // Copy the minimum page size at a time in case the physical addresses
         // are not contiguous.
         ChunkGenerator gen(pkt->dest, pkt->count, AMDGPU_MMHUB_PAGE_SIZE);
+        uint8_t *buffer_ptr = dmaBuffer;
         for (; !gen.done(); gen.next()) {
             Addr chunk_addr = getDeviceAddress(gen.addr());
             assert(chunk_addr);
@@ -738,11 +740,11 @@ SDMAEngine::copyReadData(SDMAQueue *q, sdmaCopy *pkt, uint8_t *dmaBuffer)
             DPRINTF(SDMAEngine, "Copying chunk of %d bytes to %#lx (%#lx)\n",
                     gen.size(), gen.addr(), chunk_addr);
 
-            gpuDevice->getMemMgr()->writeRequest(chunk_addr, dmaBuffer,
+            gpuDevice->getMemMgr()->writeRequest(chunk_addr, buffer_ptr,
                                                  gen.size(), 0,
                                                  gen.last() ? cb : nullptr);
 
-            dmaBuffer += gen.size();
+            buffer_ptr += gen.size();
         }
     } else {
         auto cb = new DmaVirtCallback<uint64_t>(
@@ -770,7 +772,7 @@ SDMAEngine::copyDone(SDMAQueue *q, sdmaCopy *pkt, uint8_t *dmaBuffer)
 {
     DPRINTF(SDMAEngine, "Copy completed to %p, %d dwords\n",
             pkt->dest, pkt->count);
-    delete []dmaBuffer;
+    delete [] dmaBuffer;
     delete pkt;
     decodeNext(q);
 }
@@ -1018,7 +1020,7 @@ SDMAEngine::ptePdeDone(SDMAQueue *q, sdmaPtePde *pkt, uint64_t *dmaBuffer)
     DPRINTF(SDMAEngine, "PtePde packet completed to %p, %d 2dwords\n",
             pkt->dest, pkt->count);
 
-    delete []dmaBuffer;
+    delete [] dmaBuffer;
     delete pkt;
     decodeNext(q);
 }
@@ -1108,6 +1110,7 @@ SDMAEngine::constFill(SDMAQueue *q, sdmaConstFill *pkt, uint32_t header)
         // Copy the minimum page size at a time in case the physical addresses
         // are not contiguous.
         ChunkGenerator gen(pkt->addr, fill_bytes, AMDGPU_MMHUB_PAGE_SIZE);
+        uint8_t *fill_data_ptr = fill_data;
         for (; !gen.done(); gen.next()) {
             Addr chunk_addr = getDeviceAddress(gen.addr());
             assert(chunk_addr);
@@ -1115,10 +1118,10 @@ SDMAEngine::constFill(SDMAQueue *q, sdmaConstFill *pkt, uint32_t header)
             DPRINTF(SDMAEngine, "Copying chunk of %d bytes from %#lx (%#lx)\n",
                     gen.size(), gen.addr(), chunk_addr);
 
-            gpuDevice->getMemMgr()->writeRequest(chunk_addr, fill_data,
+            gpuDevice->getMemMgr()->writeRequest(chunk_addr, fill_data_ptr,
                                                  gen.size(), 0,
                                                  gen.last() ? cb : nullptr);
-            fill_data += gen.size();
+            fill_data_ptr += gen.size();
         }
     } else {
         DPRINTF(SDMAEngine, "ConstFill %d bytes of %x to host at %lx\n",