Re: [PATCH] media: iris: fix use-after-free of fmt_src during MBPF check
From: Vishnu Reddy
Date: Mon Mar 02 2026 - 23:12:30 EST
On 2/28/2026 3:15 AM, Bryan O'Donoghue wrote:
On 27/02/2026 17:33, Vishnu Reddy wrote:During concurrency testing, multiple instances can be created in parallel.
A race condition was observed during concurrency testing. the core MBPF
check walks the list of active instances and reads fields such as fmt_src
height and width.
Where does that happen - you highlight iris_close() below - that's good, what's the method or methods that can run concurrently with iris_close() - you should state those in the commit log so that reviewers like myself and people reading the commit in the future know where to look.
At the same time, iris_close() could free these format
structures before the instance was removed from core list. this creates a
use-after-free window where the MBPF checker access the freed memory and
read invalid values.
Without looking at the code this description seems suspect.
&inst->lock ought to protect inst && inst->thing if it doesn't, then the lock isn't being used correctly.
Each instance has its own inst->lock, while the core list is protected by the global core->lock.
The race occurs because these two locks protect different scopes:
inst->lock protects fields within a single instance
core->lock protects the list of active instances
The MBPF checker walks the core instance list under the core->lock. While doing so,
it reads fields such as fmt_src->width and fmt_src->height from each instance.
Now consider what happens concurrently:
Instance A (MBPF check):
Acquires core->lock
Iterates over the instance list
Reads inst->fmt_src->width, inst->fmt_src->height, etc.
Instance B (iris_close() for a different instance):
Acquires inst->lock
Frees inst->fmt_src and inst->fmt_dst (At this moment, the instance is still in the core list.)
Releases inst->lock.
Later acquires core->lock and removes that instance from the core list
The problem is that fmt_src is freed before the instance is removed from the core list.
This creates a use‑after‑free window:
MBPF check (Instance A) is still walking the core list
It reaches the instance whose fmt_src has already been freed by Instance B
It dereferences a dangling pointer → use-after-free
This situation occurs because:
inst->lock only protects the instance internals
MBPF checker does not take inst->lock when reading fmt_src
It only relies on the core->lock, the instance is valid as long as it is in the core list
Therefore, ordering of freeing inst->fmt_src and inst->fmt_dest in iris_close() is incorrect.
The fix is to postpone freeing fmt_src and fmt_dst until after:
The instance has been removed from the core list, and
All teardown under the core lock is complete.
Will update commit description in v2.
ACK
To fix this, the freeing of fmt_src and fmt_dst is moved to the end
of iris_close(), after the instance has been removed from the core
list and teardown is complete. This avoids accessing dangling pointers
during the MBPF check.
Signed-off-by: Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
Needs
- Fixes:
- Cc: stable
Given details above and will update commit description in v2.
---
drivers/media/platform/qcom/iris/iris_vdec.c | 6 ------
drivers/media/platform/qcom/iris/iris_vdec.h | 1 -
drivers/media/platform/qcom/iris/iris_venc.c | 6 ------
drivers/media/platform/qcom/iris/iris_venc.h | 1 -
drivers/media/platform/qcom/iris/iris_vidc.c | 6 ++----
5 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index 719217399a30..99d544e2af4f 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -61,12 +61,6 @@ int iris_vdec_inst_init(struct iris_inst *inst)
return iris_ctrls_init(inst);
}
-void iris_vdec_inst_deinit(struct iris_inst *inst)
-{
- kfree(inst->fmt_dst);
- kfree(inst->fmt_src);
-}
-
static const struct iris_fmt iris_vdec_formats_cap[] = {
[IRIS_FMT_NV12] = {
.pixfmt = V4L2_PIX_FMT_NV12,
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.h b/drivers/media/platform/qcom/iris/iris_vdec.h
index ec1ce55d1375..5123d2a340e1 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.h
+++ b/drivers/media/platform/qcom/iris/iris_vdec.h
@@ -9,7 +9,6 @@
struct iris_inst;
int iris_vdec_inst_init(struct iris_inst *inst);
-void iris_vdec_inst_deinit(struct iris_inst *inst);
int iris_vdec_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f);
int iris_vdec_try_fmt(struct iris_inst *inst, struct v4l2_format *f);
int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f);
diff --git a/drivers/media/platform/qcom/iris/iris_venc.c b/drivers/media/platform/qcom/iris/iris_venc.c
index aa27b22704eb..4d886769d958 100644
--- a/drivers/media/platform/qcom/iris/iris_venc.c
+++ b/drivers/media/platform/qcom/iris/iris_venc.c
@@ -79,12 +79,6 @@ int iris_venc_inst_init(struct iris_inst *inst)
return iris_ctrls_init(inst);
}
-void iris_venc_inst_deinit(struct iris_inst *inst)
-{
- kfree(inst->fmt_dst);
- kfree(inst->fmt_src);
-}
-
static const struct iris_fmt iris_venc_formats_cap[] = {
[IRIS_FMT_H264] = {
.pixfmt = V4L2_PIX_FMT_H264,
diff --git a/drivers/media/platform/qcom/iris/iris_venc.h b/drivers/media/platform/qcom/iris/iris_venc.h
index c4db7433da53..00c1716b2747 100644
--- a/drivers/media/platform/qcom/iris/iris_venc.h
+++ b/drivers/media/platform/qcom/iris/iris_venc.h
@@ -9,7 +9,6 @@
struct iris_inst;
int iris_venc_inst_init(struct iris_inst *inst);
-void iris_venc_inst_deinit(struct iris_inst *inst);
int iris_venc_enum_fmt(struct iris_inst *inst, struct v4l2_fmtdesc *f);
int iris_venc_try_fmt(struct iris_inst *inst, struct v4l2_format *f);
int iris_venc_s_fmt(struct iris_inst *inst, struct v4l2_format *f);
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index bd38d84c9cc7..5eb1786b0737 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -289,10 +289,6 @@ int iris_close(struct file *filp)
v4l2_m2m_ctx_release(inst->m2m_ctx);
v4l2_m2m_release(inst->m2m_dev);
mutex_lock(&inst->lock);
- if (inst->domain == DECODER)
- iris_vdec_inst_deinit(inst);
- else if (inst->domain == ENCODER)
- iris_venc_inst_deinit(inst);
iris_session_close(inst);
iris_inst_change_state(inst, IRIS_INST_DEINIT);
iris_v4l2_fh_deinit(inst, filp);
@@ -304,6 +300,8 @@ int iris_close(struct file *filp)
mutex_unlock(&inst->lock);
mutex_destroy(&inst->ctx_q_lock);
mutex_destroy(&inst->lock);
+ kfree(inst->fmt_src);
+ kfree(inst->fmt_dst);
kfree(inst);
On the face of it I like the logic of moving the kfree() after destruction of the various other bits - however the description in the log makes me question of the two locks we have are being used correctly ..
Please provide more detail.
Regards,
return 0;
---
base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
change-id: 20260226-fix-use-after-free-of-fmt_src-during-mbpf-abc27f573400
Best regards,
--
Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
Vishnu Reddy