drm/amdgpu: fix heap buffer overflow in amdgpu_coredump ring dump

The off variable in the ring content dump loop tracks a byte offset
accumulated from ring->ring_size (which is in bytes), but it is used
as an index into u32 *rings_dw.  C pointer arithmetic on a u32 pointer
automatically multiplies the index by sizeof(u32) = 4, so the actual
byte address accessed is:

    &rings_dw[off]  ==  (char *)rings_dw + off * 4

This means off is effectively quadrupled, causing a 4x overshoot.

Concrete example -- two rings, each ring_size = 8 192 bytes (8 KB):

    total_ring_size = 16 384 bytes
    rings_dw = kzalloc(16 384)          /* 16 KB buffer */

  Ring 0: off = 0
    memcpy(&rings_dw[0], ring0->ring, 8192)
        -> writes bytes 0 .. 8 191                              OK

    off += ring->ring_size            -> off = 8 192   (BUG)

  Ring 1: off = 8 192
    memcpy(&rings_dw[8192], ring1->ring, 8192)
        -> actual byte offset = 8 192 * 4 = 32 768
        -> writes bytes 32 768 .. 40 959
        -> but buffer is only 16 384 bytes!             OVERFLOW

With the fix (off += ring->ring_size / 4):

  Ring 0: off = 0
    memcpy(&rings_dw[0], ring0->ring, 8192)             OK
    off += 8 192 / 4                  -> off = 2 048

  Ring 1: off = 2 048
    memcpy(&rings_dw[2048], ring1->ring, 8192)
        -> byte offset = 2 048 * 4 = 8 192
        -> writes bytes 8 192 .. 16 383                 OK

KASAN catches the overflow as a slab-use-after-free when the write
lands on a quarantined slab object:

  BUG: KASAN: slab-use-after-free in amdgpu_coredump+0x775/0x13c0 [amdgpu]
  Write of size 8192 at addr ffff8890b2400000 by task kworker/u128:1/329
  Workqueue: amdgpu-reset-dev drm_sched_job_timedout [gpu_sched]
  Call Trace:
   __asan_memcpy+0x3c/0x60
   amdgpu_coredump+0x775/0x13c0 [amdgpu]
   amdgpu_job_timedout+0xdb5/0x1420 [amdgpu]

The corrupted object was a 4 KB drm_exec buffer from a completed
amdgpu_cs_ioctl -- the ring dump memcpy overshot into this freed
slab region.

Fix by accumulating off in dword units (ring->ring_size / 4) so the
u32* indexing produces the correct byte address.  The reader in
amdgpu_devcoredump_format() already consumes the stored offset as a
dword index (rings_dw[off + j / 4]), so no change is needed there.

Fixes: eea85914d15b ("drm/amdgpu: save ring content before resetting the device")
Cc: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Jesse Zhang <jesse.zhang@amd.com>
Signed-off-by: Vitaly Prosyak <vitaly.prosyak@amd.com>
Acked-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
index 5e353a83ec0d62d1fd8b9c14e315d468f3c3b7f2..d386bc775d03c65c745c3c2921af16b74dd51748 100644 (file)
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
@@ -566,7 +566,7 @@ void amdgpu_coredump(struct amdgpu_device *adev, bool skip_vram_check,
                        coredump->rings[idx].offset = off;
 
                        memcpy(&coredump->rings_dw[off], ring->ring, ring->ring_size);
-                       off += ring->ring_size;
+                       off += ring->ring_size / 4;
                        idx++;
                }
                coredump->num_rings = idx;