Am 14.02.2017 um 10:18 schrieb Nicolai Hähnle:
From: Nicolai Hähnle nicolai.haehnle@amd.com
Fixes a potential race condition in amdgpu that looks as follows:
Task 1: attempt ttm_bo_init, but ttm_bo_validate fails Task 1: add BO to global list anyway Task 2: grabs hold of the BO, waits on its reservation lock Task 1: releases its reference of the BO; never gives up the reservation lock
The patch "drm/amdgpu: fix a potential deadlock in amdgpu_bo_create_restricted()" attempts to fix that by releasing the reservation lock in amdgpu code; unfortunately, it introduces a use-after-free when this race _doesn't_ happen.
This patch should fix the race properly by never adding the BO to the global list in the first place.
Cc: Samuel Pitoiset samuel.pitoiset@gmail.com Cc: zhoucm1 david1.zhou@amd.com Signed-off-by: Nicolai Hähnle nicolai.haehnle@amd.com
NAK, that is actually not correct either.
The previous implementation doesn't have an use after free, but actually a memory leak.
I completely agree that adding the BO to the LRU in case of an error is incorrect, but ttm_bo_add_to_lru() will add some references to the BO.
So the following ttm_bo_unref() will just drop the initial reference.
Let's clean up this mess by moving the freeing of the BO in case of an error to the caller.
Regards, Christian.
drivers/gpu/drm/ttm/ttm_bo.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 239a957..76bee42 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -1215,18 +1215,20 @@ int ttm_bo_init(struct ttm_bo_device *bdev, if (likely(!ret)) ret = ttm_bo_validate(bo, placement, interruptible, false);
- if (!resv) {
- if (!resv) ttm_bo_unreserve(bo);
- } else if (!(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
- if (unlikely(ret)) {
ttm_bo_unref(&bo);return ret;- }
- if (resv && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) { spin_lock(&bo->glob->lru_lock); ttm_bo_add_to_lru(bo); spin_unlock(&bo->glob->lru_lock); }
- if (unlikely(ret))
ttm_bo_unref(&bo);- return ret; } EXPORT_SYMBOL(ttm_bo_init);