All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()
@ 2026-06-26 16:40 ` Doruk Tan Ozturk
  0 siblings, 0 replies; 5+ messages in thread
From: Doruk Tan Ozturk @ 2026-06-26 16:40 UTC (permalink / raw)
  To: Neil Armstrong, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, linux-media, linux-amlogic, linux-staging,
	linux-arm-kernel, linux-kernel, Doruk Tan Ozturk

codec_vp9_rm_noshow_frame() frees the first non-shown reference frame on
ref_frames_list without excluding vp9->prev_frame. When the previously
decoded frame was a non-show (alt-ref) frame and the current frame is a
non-show inter frame, the freed object is the one vp9->prev_frame still
points to; codec_vp9_set_mpred_mv() then dereferences the stale pointer
(use_prev_frame_mvs and codec_vp9_get_frame_mv_paddr()), a use-after-free.

The sibling cleanup codec_vp9_show_frame() already guards this pointer
(tmp == vp9->prev_frame); rm_noshow_frame() simply omits the same check.
Add it.

The fields that drive this path (show_frame, frame_type, intra_only) are
parsed from the VP9 bitstream, so a crafted stream fed to the stateless
decoder can trigger the free-then-use.

Found by static analysis; not yet runtime-reproduced (Amlogic Meson
hardware required).

Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).

Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/staging/media/meson/vdec/codec_vp9.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
index 8e80ecf84..572f418c9 100644
--- a/drivers/staging/media/meson/vdec/codec_vp9.c
+++ b/drivers/staging/media/meson/vdec/codec_vp9.c
@@ -1247,6 +1247,15 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
 		if (tmp->show)
 			continue;
 
+		/*
+		 * prev_frame is still referenced by the MV predictor in
+		 * codec_vp9_set_mpred_mv(); the sibling codec_vp9_show_frame()
+		 * already excludes it before freeing. Do the same here to avoid
+		 * a use-after-free of vp9->prev_frame.
+		 */
+		if (tmp == vp9->prev_frame)
+			continue;
+
 		pr_debug("rm noshow: %u\n", tmp->index);
 		v4l2_m2m_buf_queue(sess->m2m_ctx, tmp->vbuf);
 		list_del(&tmp->list);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()
@ 2026-06-26 16:40 ` Doruk Tan Ozturk
  0 siblings, 0 replies; 5+ messages in thread
From: Doruk Tan Ozturk @ 2026-06-26 16:40 UTC (permalink / raw)
  To: Neil Armstrong, Greg Kroah-Hartman
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, linux-media, linux-amlogic, linux-staging,
	linux-arm-kernel, linux-kernel, Doruk Tan Ozturk

codec_vp9_rm_noshow_frame() frees the first non-shown reference frame on
ref_frames_list without excluding vp9->prev_frame. When the previously
decoded frame was a non-show (alt-ref) frame and the current frame is a
non-show inter frame, the freed object is the one vp9->prev_frame still
points to; codec_vp9_set_mpred_mv() then dereferences the stale pointer
(use_prev_frame_mvs and codec_vp9_get_frame_mv_paddr()), a use-after-free.

The sibling cleanup codec_vp9_show_frame() already guards this pointer
(tmp == vp9->prev_frame); rm_noshow_frame() simply omits the same check.
Add it.

The fields that drive this path (show_frame, frame_type, intra_only) are
parsed from the VP9 bitstream, so a crafted stream fed to the stateless
decoder can trigger the free-then-use.

Found by static analysis; not yet runtime-reproduced (Amlogic Meson
hardware required).

Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).

Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/staging/media/meson/vdec/codec_vp9.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
index 8e80ecf84..572f418c9 100644
--- a/drivers/staging/media/meson/vdec/codec_vp9.c
+++ b/drivers/staging/media/meson/vdec/codec_vp9.c
@@ -1247,6 +1247,15 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
 		if (tmp->show)
 			continue;
 
+		/*
+		 * prev_frame is still referenced by the MV predictor in
+		 * codec_vp9_set_mpred_mv(); the sibling codec_vp9_show_frame()
+		 * already excludes it before freeing. Do the same here to avoid
+		 * a use-after-free of vp9->prev_frame.
+		 */
+		if (tmp == vp9->prev_frame)
+			continue;
+
 		pr_debug("rm noshow: %u\n", tmp->index);
 		v4l2_m2m_buf_queue(sess->m2m_ctx, tmp->vbuf);
 		list_del(&tmp->list);
-- 
2.43.0


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()
  2026-06-26 16:40 ` Doruk Tan Ozturk
  (?)
@ 2026-06-26 16:58 ` sashiko-bot
  -1 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-26 16:58 UTC (permalink / raw)
  To: Doruk Tan Ozturk; +Cc: neil.armstrong, linux-amlogic

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Flushing the decoder leaves a dangling pointer in `vp9->prev_frame`, leading to a Use-After-Free.
- [High] Deleting active reference frames causes hardware desync when configuring reference canvas data.
- [High] Kernel NULL pointer dereference in `codec_vp9_set_mpred_mv()` if a stream begins with a non-key frame or after a resolution change.
- [High] An infinite loop in `codec_vp9_get_new_frame()` causes a hard kernel lockup if the ready queue only contains buffers currently used as reference frames.
--

commit 6e91e5b5beb7355817230e0312560c02d6e0441c
Author: Doruk Tan Ozturk <doruk@0sec.ai>
Subject: media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()

This commit adds a check in codec_vp9_rm_noshow_frame() to avoid freeing
vp9->prev_frame. It prevents a use-after-free regression that occurs when the
current frame is a non-show inter frame and the previous decoded frame was an
alt-ref frame.

> diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
> --- a/drivers/staging/media/meson/vdec/codec_vp9.c
> +++ b/drivers/staging/media/meson/vdec/codec_vp9.c
> @@ -1247,6 +1247,15 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
>  		if (tmp->show)
>  			continue;

[Severity: High]
This is a pre-existing issue, but does this loop delete active reference frames
without consulting codec_vp9_is_ref()?

If a stream contains multiple non-show frames and one is an active reference,
it appears it is unconditionally deleted here. Later, codec_vp9_sync_ref()
could fail to find it, leaving vp9->frame_refs array with missing entries.

This would cause codec_vp9_set_refs() to skip writing to the auto-incrementing
HEVCD_MPP_ANC_CANVAS_DATA_ADDR hardware port:

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_set_refs() {
    ...
    if (!frame)
        continue;
    ...
    amvdec_write_dos(core, HEVCD_MPP_ANC_CANVAS_DATA_ADDR,
    ...
}

Could this missing write desync the hardware configuration sequence and
corrupt the decoding?

> +		/*
> +		 * prev_frame is still referenced by the MV predictor in
> +		 * codec_vp9_set_mpred_mv(); the sibling codec_vp9_show_frame()
> +		 * already excludes it before freeing. Do the same here to avoid
> +		 * a use-after-free of vp9->prev_frame.
> +		 */
> +		if (tmp == vp9->prev_frame)
> +			continue;
> +
>  		pr_debug("rm noshow: %u\n", tmp->index);
>  		v4l2_m2m_buf_queue(sess->m2m_ctx, tmp->vbuf);
>  		list_del(&tmp->list);


[Severity: High]
This is a pre-existing issue, but is there a risk of a null pointer
dereference in codec_vp9_set_mpred_mv() if the stream begins with a
non-key frame?

In codec_vp9_process_frame(), codec_vp9_set_mpred_mv() is called based on
the frame type:

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_process_frame() {
    ...
    if (param->p.frame_type != KEY_FRAME && !intra_only) {
        codec_vp9_set_mc(sess, vp9);
        codec_vp9_set_mpred_mv(core, vp9);
    }
    ...
}

If vp9->prev_frame is null because a keyframe was missing, wouldn't the
unconditional dereference in codec_vp9_set_mpred_mv() cause a crash?

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_set_mpred_mv() {
    ...
    int use_prev_frame_mvs = vp9->prev_frame->width ==
                             vp9->cur_frame->width &&
                             vp9->prev_frame->height ==
    ...
}


[Severity: High]
This is a pre-existing issue, but could codec_vp9_get_new_frame() get stuck
in an infinite loop if the ready queue only contains buffers currently used
as reference frames?

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_get_new_frame() {
    ...
    while (codec_vp9_get_frame_by_idx(vp9, vbuf->vb2_buf.index)) {
        struct vb2_v4l2_buffer *old_vbuf = vbuf;

        vbuf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx);
        v4l2_m2m_buf_queue(sess->m2m_ctx, old_vbuf);
    ...
}

If userspace immediately queues back buffers that are still held by the driver
as active reference frames, and no other buffers are in the queue, does this
continually pop and push the same referenced buffers forever?


[Severity: Critical]
This is a pre-existing issue, but does flushing the decoder leave a
dangling pointer in vp9->prev_frame?

When flushing, codec_vp9_flush_output() frees the reference frames list:

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_flush_output() {
    ...
    list_for_each_entry_safe(tmp, n, &vp9->ref_frames_list, list) {
        ...
        list_del(&tmp->list);
        kfree(tmp);
    }
    ...
}

Since vp9->prev_frame is not cleared here, if decoding is subsequently
resumed with an INTER_FRAME, codec_vp9_resume() would process it and call
codec_vp9_set_mpred_mv(), which dereferences vp9->prev_frame:

drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_set_mpred_mv() {
    ...
    int use_prev_frame_mvs = vp9->prev_frame->width ==
                             vp9->cur_frame->width &&
    ...
}

Could this sequence lead to accessing freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626164025.52694-1-doruk@0sec.ai?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()
  2026-06-26 16:40 ` Doruk Tan Ozturk
@ 2026-06-26 17:29   ` Dan Carpenter
  -1 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2026-06-26 17:29 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: Neil Armstrong, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	Hans Verkuil, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	linux-media, linux-amlogic, linux-staging, linux-arm-kernel,
	linux-kernel

On Fri, Jun 26, 2026 at 06:40:25PM +0200, Doruk Tan Ozturk wrote:
> codec_vp9_rm_noshow_frame() frees the first non-shown reference frame on
> ref_frames_list without excluding vp9->prev_frame. When the previously
> decoded frame was a non-show (alt-ref) frame and the current frame is a
> non-show inter frame, the freed object is the one vp9->prev_frame still
> points to; codec_vp9_set_mpred_mv() then dereferences the stale pointer
> (use_prev_frame_mvs and codec_vp9_get_frame_mv_paddr()), a use-after-free.
> 
> The sibling cleanup codec_vp9_show_frame() already guards this pointer
> (tmp == vp9->prev_frame); rm_noshow_frame() simply omits the same check.
> Add it.
> 
> The fields that drive this path (show_frame, frame_type, intra_only) are
> parsed from the VP9 bitstream, so a crafted stream fed to the stateless
> decoder can trigger the free-then-use.
> 
> Found by static analysis; not yet runtime-reproduced (Amlogic Meson
> hardware required).
> 
> Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
> 
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
>  drivers/staging/media/meson/vdec/codec_vp9.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
> index 8e80ecf84..572f418c9 100644
> --- a/drivers/staging/media/meson/vdec/codec_vp9.c
> +++ b/drivers/staging/media/meson/vdec/codec_vp9.c
> @@ -1247,6 +1247,15 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
>  		if (tmp->show)
>  			continue;
>  
> +		/*
> +		 * prev_frame is still referenced by the MV predictor in
> +		 * codec_vp9_set_mpred_mv(); the sibling codec_vp9_show_frame()
> +		 * already excludes it before freeing. Do the same here to avoid
> +		 * a use-after-free of vp9->prev_frame.
> +		 */
> +		if (tmp == vp9->prev_frame)
> +			continue;

I have not looked at this code before so I'm speaking from a position
of ignorance but codec_vp9_show_frame() checks vp9->cur_frame as well.
Shouldn't we check that here as well?

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame()
@ 2026-06-26 17:29   ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2026-06-26 17:29 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: Neil Armstrong, Greg Kroah-Hartman, Mauro Carvalho Chehab,
	Hans Verkuil, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
	linux-media, linux-amlogic, linux-staging, linux-arm-kernel,
	linux-kernel

On Fri, Jun 26, 2026 at 06:40:25PM +0200, Doruk Tan Ozturk wrote:
> codec_vp9_rm_noshow_frame() frees the first non-shown reference frame on
> ref_frames_list without excluding vp9->prev_frame. When the previously
> decoded frame was a non-show (alt-ref) frame and the current frame is a
> non-show inter frame, the freed object is the one vp9->prev_frame still
> points to; codec_vp9_set_mpred_mv() then dereferences the stale pointer
> (use_prev_frame_mvs and codec_vp9_get_frame_mv_paddr()), a use-after-free.
> 
> The sibling cleanup codec_vp9_show_frame() already guards this pointer
> (tmp == vp9->prev_frame); rm_noshow_frame() simply omits the same check.
> Add it.
> 
> The fields that drive this path (show_frame, frame_type, intra_only) are
> parsed from the VP9 bitstream, so a crafted stream fed to the stateless
> decoder can trigger the free-then-use.
> 
> Found by static analysis; not yet runtime-reproduced (Amlogic Meson
> hardware required).
> 
> Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
> 
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
>  drivers/staging/media/meson/vdec/codec_vp9.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
> index 8e80ecf84..572f418c9 100644
> --- a/drivers/staging/media/meson/vdec/codec_vp9.c
> +++ b/drivers/staging/media/meson/vdec/codec_vp9.c
> @@ -1247,6 +1247,15 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
>  		if (tmp->show)
>  			continue;
>  
> +		/*
> +		 * prev_frame is still referenced by the MV predictor in
> +		 * codec_vp9_set_mpred_mv(); the sibling codec_vp9_show_frame()
> +		 * already excludes it before freeing. Do the same here to avoid
> +		 * a use-after-free of vp9->prev_frame.
> +		 */
> +		if (tmp == vp9->prev_frame)
> +			continue;

I have not looked at this code before so I'm speaking from a position
of ignorance but codec_vp9_show_frame() checks vp9->cur_frame as well.
Shouldn't we check that here as well?

regards,
dan carpenter


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-26 17:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26 16:40 [PATCH] media: meson: vdec: fix use-after-free of prev_frame in codec_vp9_rm_noshow_frame() Doruk Tan Ozturk
2026-06-26 16:40 ` Doruk Tan Ozturk
2026-06-26 16:58 ` sashiko-bot
2026-06-26 17:29 ` Dan Carpenter
2026-06-26 17:29   ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.