]> git.dujemihanovic.xyz Git - linux.git/commitdiff
drm/xe/client: add missing bo locking in show_meminfo()
authorMatthew Auld <matthew.auld@intel.com>
Wed, 11 Sep 2024 15:55:28 +0000 (16:55 +0100)
committerRodrigo Vivi <rodrigo.vivi@intel.com>
Thu, 12 Sep 2024 14:07:22 +0000 (10:07 -0400)
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: 0845233388f8 ("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 4f63d712fa104c3ebefcb289d1e733e86d8698c7)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
drivers/gpu/drm/xe/xe_drm_client.c

index ec141357480f21266e652a84eae77a8e53f74d4b..1af95b9b917156ef92c8e9245564a9622af3ca30 100644 (file)
@@ -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"
@@ -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);
@@ -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
@@ -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);
 
@@ -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);