Skip to content

Commit 7f9cf7f

Browse files
Patrick Lerdadcbaker-intel
Patrick Lerda
authored andcommitted
winsys/radeon: fix radeon_winsys_bo_from_handle() related race condition
This change prevents the reuse of the bo when the counter is already zero. At zero, the bo is in a state where the deletion is pending, and this implementation relying on an atomic counter can't be safely stopped. In other words, the previous fix ccd3bb4 lower the probability of this race condition, but doesn't fix it. This change prevents a race condition which has a high probability on r600 with the test below. This change was tested with the thread sanitizer. For instance, this issue is triggered on r600 with "piglit/bin/ext_image_dma_buf_import-refcount-multithread -auto": ==9876==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000021a20 at pc 0x7f2c9f59f748 bp 0x7f2c8f3aa600 sp 0x7f2c8f3aa5f8 READ of size 4 at 0x60d000021a20 thread T6 #0 0x7f2c9f59f747 in pipe_is_referenced ../src/gallium/auxiliary/util/u_inlines.h:65 waydroid#1 0x7f2c9f59f747 in radeon_bo_destroy ../src/gallium/winsys/radeon/drm/radeon_drm_bo.c:342 waydroid#2 0x7f2c9f63b541 in radeon_bo_reference ../src/gallium/include/winsys/radeon_winsys.h:794 waydroid#3 0x7f2c9f63b541 in r600_texture_destroy ../src/gallium/drivers/r600/r600_texture.c:571 waydroid#4 0x7f2c9d65662d in pipe_resource_destroy ../src/gallium/auxiliary/util/u_inlines.h:146 waydroid#5 0x7f2c9d65662d in pipe_resource_reference ../src/gallium/auxiliary/util/u_inlines.h:163 waydroid#6 0x7f2c9d65662d in st_FreeTextureImageBuffer ../src/mesa/state_tracker/st_cb_texture.c:459 waydroid#7 0x7f2c9d5b6991 in _mesa_delete_texture_image ../src/mesa/main/teximage.c:226 waydroid#8 0x7f2c9d5f2593 in _mesa_delete_texture_object ../src/mesa/main/texobj.c:532 waydroid#9 0x7f2c9d5f2be7 in _mesa_reference_texobj_ ../src/mesa/main/texobj.c:639 waydroid#10 0x7f2c9d5f3773 in _mesa_reference_texobj ../src/mesa/main/texobj.h:92 waydroid#11 0x7f2c9d5f3773 in delete_textures ../src/mesa/main/texobj.c:1578 0x60d000021a20 is located 0 bytes inside of 144-byte region [0x60d000021a20,0x60d000021ab0) freed by thread T5 here: #0 0x7f2ca8b2b4f7 in free (/usr/lib64/libasan.so.6+0xb14f7) waydroid#1 0x7f2c9f59efb3 in radeon_bo_destroy ../src/gallium/winsys/radeon/drm/radeon_drm_bo.c:401 waydroid#2 0x7f2c9f63b541 in radeon_bo_reference ../src/gallium/include/winsys/radeon_winsys.h:794 waydroid#3 0x7f2c9f63b541 in r600_texture_destroy ../src/gallium/drivers/r600/r600_texture.c:571 waydroid#4 0x7f2c9d65662d in pipe_resource_destroy ../src/gallium/auxiliary/util/u_inlines.h:146 waydroid#5 0x7f2c9d65662d in pipe_resource_reference ../src/gallium/auxiliary/util/u_inlines.h:163 waydroid#6 0x7f2c9d65662d in st_FreeTextureImageBuffer ../src/mesa/state_tracker/st_cb_texture.c:459 waydroid#7 0x7f2c9d5b6991 in _mesa_delete_texture_image ../src/mesa/main/teximage.c:226 waydroid#8 0x7f2c9d5f2593 in _mesa_delete_texture_object ../src/mesa/main/texobj.c:532 waydroid#9 0x7f2c9d5f2be7 in _mesa_reference_texobj_ ../src/mesa/main/texobj.c:639 waydroid#10 0x7f2c9d5f3773 in _mesa_reference_texobj ../src/mesa/main/texobj.h:92 waydroid#11 0x7f2c9d5f3773 in delete_textures ../src/mesa/main/texobj.c:1578 previously allocated by thread T6 here: #0 0x7f2ca8b2b9a7 in calloc (/usr/lib64/libasan.so.6+0xb19a7) waydroid#1 0x7f2c9f5a36d5 in radeon_winsys_bo_from_handle ../src/gallium/winsys/radeon/drm/radeon_drm_bo.c:1198 waydroid#2 0x7f2c9f641b2a in r600_texture_from_handle ../src/gallium/drivers/r600/r600_texture.c:1105 waydroid#3 0x7f2c9d47550a in dri_create_image_from_winsys ../src/gallium/frontends/dri/dri2.c:1007 waydroid#4 0x7f2c9d47eeb9 in dri2_from_dma_bufs ../src/gallium/frontends/dri/dri2.c:1629 waydroid#5 0x7f2ca8854360 in dri2_create_image_dma_buf ../src/egl/drivers/dri2/egl_dri2.c:2564 waydroid#6 0x7f2ca8854f45 in dri2_create_image_khr ../src/egl/drivers/dri2/egl_dri2.c:2817 waydroid#7 0x7f2ca8846f2c in dri2_create_image ../src/egl/drivers/dri2/egl_dri2.c:1864 waydroid#8 0x7f2ca87f9dd8 in _eglCreateImageCommon ../src/egl/main/eglapi.c:1850 Fixes: ccd3bb4 ("winsys/radeon: fix a race between bo import and destroy") Signed-off-by: Patrick Lerda <[email protected]> Reviewed-by: Marek Olšák <[email protected]> (cherry picked from commit c6bcf88) Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33113>
1 parent e777076 commit 7f9cf7f

File tree

2 files changed

+7
-8
lines changed

2 files changed

+7
-8
lines changed

.pick_status.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@
10041004
"description": "winsys/radeon: fix radeon_winsys_bo_from_handle() related race condition",
10051005
"nominated": true,
10061006
"nomination_type": 2,
1007-
"resolution": 0,
1007+
"resolution": 1,
10081008
"main_sha": null,
10091009
"because_sha": "ccd3bb45483b25330f435d7e041a69237edc9631",
10101010
"notes": null

src/gallium/winsys/radeon/drm/radeon_drm_bo.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,6 @@ void radeon_bo_destroy(void *winsys, struct pb_buffer_lean *_buf)
338338
memset(&args, 0, sizeof(args));
339339

340340
mtx_lock(&rws->bo_handles_mutex);
341-
/* radeon_winsys_bo_from_handle might have revived the bo */
342-
if (pipe_is_referenced(&bo->base.reference)) {
343-
mtx_unlock(&rws->bo_handles_mutex);
344-
return;
345-
}
346341
_mesa_hash_table_remove_key(rws->bo_handles, (void*)(uintptr_t)bo->handle);
347342
if (bo->flink_name) {
348343
_mesa_hash_table_remove_key(rws->bo_names,
@@ -1190,8 +1185,12 @@ static struct pb_buffer_lean *radeon_winsys_bo_from_handle(struct radeon_winsys
11901185

11911186
if (bo) {
11921187
/* Increase the refcount. */
1193-
p_atomic_inc(&bo->base.reference.count);
1194-
goto done;
1188+
if (unlikely(p_atomic_inc_return(&bo->base.reference.count) == 1)) {
1189+
p_atomic_dec(&bo->base.reference.count);
1190+
assert(p_atomic_read(&bo->base.reference.count) == 0);
1191+
} else {
1192+
goto done;
1193+
}
11951194
}
11961195

11971196
/* There isn't, create a new one. */

0 commit comments

Comments
 (0)