From f5f9e42e5ef904e14c841e025e376913496cbeb8 Mon Sep 17 00:00:00 2001 From: Pierre-Eric Pelloux-Prayer Date: Mon, 17 May 2021 18:41:26 +0200 Subject: [PATCH] winsys/amdgpu: don't read bo->u.slab.entry after pb_slab_free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise the pb_slabs might be freed by another thread in between. Valgrind example: ==676841== Invalid read of size 1 ==676841== at 0x6B0A8B3: get_slab_wasted_size (amdgpu_bo.c:659) ==676841== by 0x6B0AD7D: amdgpu_bo_slab_destroy (amdgpu_bo.c:684) ==676841== by 0x6ACF94F: pb_destroy (pb_buffer.h:259) ==676841== by 0x6ACF94F: pb_reference_with_winsys (pb_buffer.h:282) ==676841== by 0x6ACF94F: radeon_bo_reference (radeon_winsys.h:754) ==676841== by 0x6ACF94F: si_replace_buffer_storage (si_buffer.c:274) ==676841== by 0x6957036: tc_call_replace_buffer_storage (u_threaded_context.c:1554) [...] ==676841== by 0x4ECCDEE: clone (clone.S:95) ==676841== Address 0x27879945 is 5 bytes inside a block of size 208 free'd ==676841== at 0x48399AB: free (vg_replace_malloc.c:538) ==676841== by 0x6B0E8BD: amdgpu_bo_slab_free (amdgpu_bo.c:863) ==676841== by 0x6B89D4A: pb_slabs_reclaim_locked (pb_slab.c:84) ==676841== by 0x6B89D4A: pb_slab_alloc (pb_slab.c:130) ==676841== by 0x6B0EE7F: amdgpu_bo_create (amdgpu_bo.c:1429) Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4736 Fixes: 965c6445ad4 ("winsys/amdgpu,radeonsi: add HUD counters for how much memory is wasted by slabs") Reviewed-by: Marek Olšák Part-of: (cherry picked from commit 1bd64d8cfbc1034593c912c77e1a0403ac9007db) --- .pick_status.json | 2 +- src/gallium/winsys/amdgpu/drm/amdgpu_bo.c | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 37d8be420c9..e06472f5d1f 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1579,7 +1579,7 @@ "description": "winsys/amdgpu: don't read bo->u.slab.entry after pb_slab_free", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "965c6445ad419aa49302f75db1d99345708c5aae" }, diff --git a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c index 6cadfaa4313..ba98aa13075 100644 --- a/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c +++ b/src/gallium/winsys/amdgpu/drm/amdgpu_bo.c @@ -666,22 +666,18 @@ static void amdgpu_bo_slab_destroy(struct radeon_winsys *rws, struct pb_buffer * { struct amdgpu_winsys *ws = amdgpu_winsys(rws); struct amdgpu_winsys_bo *bo = amdgpu_winsys_bo(_buf); + struct pb_slabs *slabs; assert(!bo->bo); - if (bo->base.usage & RADEON_FLAG_ENCRYPTED) - pb_slab_free(get_slabs(ws, - bo->base.size, - RADEON_FLAG_ENCRYPTED), &bo->u.slab.entry); - else - pb_slab_free(get_slabs(ws, - bo->base.size, - 0), &bo->u.slab.entry); + slabs = get_slabs(ws, bo->base.size, bo->base.usage & RADEON_FLAG_ENCRYPTED); if (bo->base.placement & RADEON_DOMAIN_VRAM) ws->slab_wasted_vram -= get_slab_wasted_size(ws, bo); else ws->slab_wasted_gtt -= get_slab_wasted_size(ws, bo); + + pb_slab_free(slabs, &bo->u.slab.entry); } static const struct pb_vtbl amdgpu_winsys_bo_slab_vtbl = {