On Tue, Sep 10, 2024 at 02:11:47PM +0100, Matthew Auld wrote:
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.
Path LGTM, one suggestion.
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+
drivers/gpu/drm/xe/xe_drm_client.c | 37 +++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c index badfa045ead8..3cca741c500c 100644 --- a/drivers/gpu/drm/xe/xe_drm_client.c +++ b/drivers/gpu/drm/xe/xe_drm_client.c @@ -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); @@ -207,7 +211,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);
Add a xe_bo_assert_held to bo_meminfo.
With that exrta assert: Reviewed-by: Matthew Brost matthew.brost@intel.com
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);}@@ -217,7 +234,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);
-- 2.46.0