Skip to content

Commit fe69a39

Browse files
danvetSteven Price
authored andcommitted
drm/panthor: Fix UAF in panthor_gem_create_with_handle() debugfs code
The object is potentially already gone after the drm_gem_object_put(). In general the object should be fully constructed before calling drm_gem_handle_create(), except the debugfs tracking uses a separate lock and list and separate flag to denotate whether the object is actually initialized. Since I'm touching this all anyway simplify this by only adding the object to the debugfs when it's ready for that, which allows us to delete that separate flag. panthor_gem_debugfs_bo_rm() already checks whether we've actually been added to the list or this is some error path cleanup. v2: Fix build issues for !CONFIG_DEBUGFS (Adrián) v3: Add linebreak and remove outdated comment (Liviu) Fixes: a3707f5 ("drm/panthor: show device-wide list of DRM GEM objects over DebugFS") Cc: Adrián Larumbe <[email protected]> Cc: Boris Brezillon <[email protected]> Cc: Steven Price <[email protected]> Cc: Liviu Dudau <[email protected]> Reviewed-by: Liviu Dudau <[email protected]> Signed-off-by: Simona Vetter <[email protected]> Signed-off-by: Simona Vetter <[email protected]> Reviewed-by: Steven Price <[email protected]> Signed-off-by: Steven Price <[email protected]> Link: https://lore.kernel.org/r/[email protected]
1 parent 0f168e7 commit fe69a39

File tree

2 files changed

+15
-19
lines changed

2 files changed

+15
-19
lines changed

drivers/gpu/drm/panthor/panthor_gem.c

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,15 @@
1616
#include "panthor_mmu.h"
1717

1818
#ifdef CONFIG_DEBUG_FS
19-
static void panthor_gem_debugfs_bo_add(struct panthor_device *ptdev,
20-
struct panthor_gem_object *bo)
19+
static void panthor_gem_debugfs_bo_init(struct panthor_gem_object *bo)
2120
{
2221
INIT_LIST_HEAD(&bo->debugfs.node);
22+
}
23+
24+
static void panthor_gem_debugfs_bo_add(struct panthor_gem_object *bo)
25+
{
26+
struct panthor_device *ptdev = container_of(bo->base.base.dev,
27+
struct panthor_device, base);
2328

2429
bo->debugfs.creator.tgid = current->group_leader->pid;
2530
get_task_comm(bo->debugfs.creator.process_name, current->group_leader);
@@ -44,14 +49,13 @@ static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo)
4449

4550
static void panthor_gem_debugfs_set_usage_flags(struct panthor_gem_object *bo, u32 usage_flags)
4651
{
47-
bo->debugfs.flags = usage_flags | PANTHOR_DEBUGFS_GEM_USAGE_FLAG_INITIALIZED;
52+
bo->debugfs.flags = usage_flags;
53+
panthor_gem_debugfs_bo_add(bo);
4854
}
4955
#else
50-
static void panthor_gem_debugfs_bo_add(struct panthor_device *ptdev,
51-
struct panthor_gem_object *bo)
52-
{}
5356
static void panthor_gem_debugfs_bo_rm(struct panthor_gem_object *bo) {}
5457
static void panthor_gem_debugfs_set_usage_flags(struct panthor_gem_object *bo, u32 usage_flags) {}
58+
static void panthor_gem_debugfs_bo_init(struct panthor_gem_object *bo) {}
5559
#endif
5660

5761
static void panthor_gem_free_object(struct drm_gem_object *obj)
@@ -246,7 +250,7 @@ struct drm_gem_object *panthor_gem_create_object(struct drm_device *ddev, size_t
246250
drm_gem_gpuva_set_lock(&obj->base.base, &obj->gpuva_list_lock);
247251
mutex_init(&obj->label.lock);
248252

249-
panthor_gem_debugfs_bo_add(ptdev, obj);
253+
panthor_gem_debugfs_bo_init(obj);
250254

251255
return &obj->base.base;
252256
}
@@ -285,6 +289,8 @@ panthor_gem_create_with_handle(struct drm_file *file,
285289
bo->base.base.resv = bo->exclusive_vm_root_gem->resv;
286290
}
287291

292+
panthor_gem_debugfs_set_usage_flags(bo, 0);
293+
288294
/*
289295
* Allocate an id of idr table where the obj is registered
290296
* and handle has the id what user can see.
@@ -296,12 +302,6 @@ panthor_gem_create_with_handle(struct drm_file *file,
296302
/* drop reference from allocate - handle holds it now. */
297303
drm_gem_object_put(&shmem->base);
298304

299-
/*
300-
* No explicit flags are needed in the call below, since the
301-
* function internally sets the INITIALIZED bit for us.
302-
*/
303-
panthor_gem_debugfs_set_usage_flags(bo, 0);
304-
305305
return ret;
306306
}
307307

@@ -387,7 +387,7 @@ static void panthor_gem_debugfs_bo_print(struct panthor_gem_object *bo,
387387
unsigned int refcount = kref_read(&bo->base.base.refcount);
388388
char creator_info[32] = {};
389389
size_t resident_size;
390-
u32 gem_usage_flags = bo->debugfs.flags & (u32)~PANTHOR_DEBUGFS_GEM_USAGE_FLAG_INITIALIZED;
390+
u32 gem_usage_flags = bo->debugfs.flags;
391391
u32 gem_state_flags = 0;
392392

393393
/* Skip BOs being destroyed. */
@@ -436,8 +436,7 @@ void panthor_gem_debugfs_print_bos(struct panthor_device *ptdev,
436436

437437
scoped_guard(mutex, &ptdev->gems.lock) {
438438
list_for_each_entry(bo, &ptdev->gems.node, debugfs.node) {
439-
if (bo->debugfs.flags & PANTHOR_DEBUGFS_GEM_USAGE_FLAG_INITIALIZED)
440-
panthor_gem_debugfs_bo_print(bo, m, &totals);
439+
panthor_gem_debugfs_bo_print(bo, m, &totals);
441440
}
442441
}
443442

drivers/gpu/drm/panthor/panthor_gem.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ enum panthor_debugfs_gem_usage_flags {
3535

3636
/** @PANTHOR_DEBUGFS_GEM_USAGE_FLAG_FW_MAPPED: BO is mapped on the FW VM. */
3737
PANTHOR_DEBUGFS_GEM_USAGE_FLAG_FW_MAPPED = BIT(PANTHOR_DEBUGFS_GEM_USAGE_FW_MAPPED_BIT),
38-
39-
/** @PANTHOR_DEBUGFS_GEM_USAGE_FLAG_INITIALIZED: BO is ready for DebugFS display. */
40-
PANTHOR_DEBUGFS_GEM_USAGE_FLAG_INITIALIZED = BIT(31),
4138
};
4239

4340
/**

0 commit comments

Comments
 (0)