Skip to content

Commit

Permalink
drm/xe/client: add missing bo locking in show_meminfo()
Browse files Browse the repository at this point in the history
bo_meminfo() wants to inspect bo state like tt and the ttm resource,
however this state can change at any point leading to stuff like NPD and
UAF, if the bo lock is not held. Grab the bo lock when calling
bo_meminfo(), ensuring we drop any spinlocks first. In the case of
object_idr we now also need to hold a ref.

v2 (MattB)
  - Also add xe_bo_assert_held()

Fixes: 0845233 ("drm/xe: Implement fdinfo memory stats printing")
Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: <stable@vger.kernel.org> # v6.8+
Reviewed-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Tejas Upadhyay <tejas.upadhyay@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20240911155527.178910-6-matthew.auld@intel.com
(cherry picked from commit 4f63d71)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
  • Loading branch information
Matthew Auld authored and Rodrigo Vivi committed Sep 12, 2024
1 parent 9bd7ff2 commit 94c4aa2
Showing 1 changed file with 36 additions and 3 deletions.
39 changes: 36 additions & 3 deletions drivers/gpu/drm/xe/xe_drm_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <linux/slab.h>
#include <linux/types.h>

#include "xe_assert.h"
#include "xe_bo.h"
#include "xe_bo_types.h"
#include "xe_device_types.h"
Expand Down Expand Up @@ -151,10 +152,13 @@ void xe_drm_client_add_bo(struct xe_drm_client *client,
*/
void xe_drm_client_remove_bo(struct xe_bo *bo)
{
struct xe_device *xe = ttm_to_xe_device(bo->ttm.bdev);
struct xe_drm_client *client = bo->client;

xe_assert(xe, !kref_read(&bo->ttm.base.refcount));

spin_lock(&client->bos_lock);
list_del(&bo->client_link);
list_del_init(&bo->client_link);
spin_unlock(&client->bos_lock);

xe_drm_client_put(client);
Expand All @@ -166,6 +170,8 @@ static void bo_meminfo(struct xe_bo *bo,
u64 sz = bo->size;
u32 mem_type;

xe_bo_assert_held(bo);

if (bo->placement.placement)
mem_type = bo->placement.placement->mem_type;
else
Expand Down Expand Up @@ -207,7 +213,20 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
idr_for_each_entry(&file->object_idr, obj, id) {
struct xe_bo *bo = gem_to_xe_bo(obj);

bo_meminfo(bo, stats);
if (dma_resv_trylock(bo->ttm.base.resv)) {
bo_meminfo(bo, stats);
xe_bo_unlock(bo);
} else {
xe_bo_get(bo);
spin_unlock(&file->table_lock);

xe_bo_lock(bo, false);
bo_meminfo(bo, stats);
xe_bo_unlock(bo);

xe_bo_put(bo);
spin_lock(&file->table_lock);
}
}
spin_unlock(&file->table_lock);

Expand All @@ -217,7 +236,21 @@ static void show_meminfo(struct drm_printer *p, struct drm_file *file)
if (!kref_get_unless_zero(&bo->ttm.base.refcount))
continue;

bo_meminfo(bo, stats);
if (dma_resv_trylock(bo->ttm.base.resv)) {
bo_meminfo(bo, stats);
xe_bo_unlock(bo);
} else {
spin_unlock(&client->bos_lock);

xe_bo_lock(bo, false);
bo_meminfo(bo, stats);
xe_bo_unlock(bo);

spin_lock(&client->bos_lock);
/* The bo ref will prevent this bo from being removed from the list */
xe_assert(xef->xe, !list_empty(&bo->client_link));
}

xe_bo_put_deferred(bo, &deferred);
}
spin_unlock(&client->bos_lock);
Expand Down

0 comments on commit 94c4aa2

Please sign in to comment.