On 02/04/2026 19:36, Maíra Canal wrote:
This is a preparation commit for per-file descriptor seqno tracking,
which will require the struct vc4_render_job to store a pointer to
vc4_file. Since the scheduler's free_job callback runs asynchronously,
it can execute after vc4_close() has already freed the vc4_file, causing
a use-after-free.
Add kref-based reference counting to vc4_file so that jobs can safely
take a reference at submit time via vc4_file_get() and drop it in their
free callback via vc4_file_put().
Signed-off-by: Maíra Canal <[email protected]>
---
drivers/gpu/drm/vc4/vc4_drv.c | 23 +++++++++++++++++++++--
drivers/gpu/drm/vc4/vc4_drv.h | 4 ++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index
cacd4cb942593b44f2210b7738bd964ebc34522d..d0e515c900af6e7400a96e42a9141dab5d5bca2d
100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -167,6 +167,7 @@ static int vc4_open(struct drm_device *dev, struct drm_file
*file)
goto err_sched;
}
+ kref_init(&vc4file->refcount);
vc4_perfmon_open_file(vc4file);
file->driver_priv = vc4file;
@@ -180,6 +181,25 @@ static int vc4_open(struct drm_device *dev, struct drm_file *file)
}
+static void vc4_file_release(struct kref *ref)
+{
+ struct vc4_file *vc4file = container_of(ref, struct vc4_file, refcount);
+
+ vc4_perfmon_close_file(vc4file);
Access of vc4->active_perfmon is not clear to me. When I was looking at
the earlier patches I assumed it is protected by the run_job /
workqueue. But from the angle of a delayed vc4_file release I am not
sure why it is safe.
vc4_perfmon_close_file
-> vc4_perfmon_delete
/* If the active perfmon is being destroyed, stop it first */
if (perfmon == vc4->active_perfmon)
vc4_perfmon_stop(vc4, perfmon, false);
Why is vc4->active_perfmon stable here? Something guarantees the
perfmons belonging to this file (vc4file->perfmons) will not be in the
xarray any longer if it was the active one?
+ kfree(vc4file);
+}
+
+struct vc4_file *vc4_file_get(struct vc4_file *vc4file)
+{
+ kref_get(&vc4file->refcount);
+ return vc4file;
+}
+
+void vc4_file_put(struct vc4_file *vc4file)
+{
+ kref_put(&vc4file->refcount, vc4_file_release);
+}
I'd be temped to "static inline" the above two in a header file
(avoiding trivial function calls) but up to you.
Regards,
Tvrtko
+
static void vc4_close(struct drm_device *dev, struct drm_file *file)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -195,8 +215,7 @@ static void vc4_close(struct drm_device *dev, struct
drm_file *file)
for (q = 0; q < VC4_MAX_QUEUES; q++)
drm_sched_entity_destroy(&vc4file->sched_entity[q]);
- vc4_perfmon_close_file(vc4file);
- kfree(vc4file);
+ vc4_file_put(vc4file);
}
DEFINE_DRM_GEM_FOPS(vc4_drm_fops);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index
b828c56f98965944b94a330e6c4e5ff95c9ab8f8..2a331dd053cab1e55476cb028f399a5d25eb4309
100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -898,6 +898,8 @@ struct vc4_exec_info {
struct vc4_file {
struct vc4_dev *dev;
+ struct kref refcount;
+
struct xarray perfmons;
struct drm_sched_entity sched_entity[VC4_MAX_QUEUES];
@@ -1083,6 +1085,8 @@ static inline void vc4_debugfs_add_regset32(struct
drm_device *drm,
#endif
/* vc4_drv.c */
+struct vc4_file *vc4_file_get(struct vc4_file *vc4file);
+void vc4_file_put(struct vc4_file *vc4file);
void __iomem *vc4_ioremap_regs(struct platform_device *dev, int index);
int vc4_dumb_fixup_args(struct drm_mode_create_dumb *args);
