Re: [PATCH v2] media: meson: vdec: fix use-after-free of in-use frames in codec_vp9_rm_noshow_frame()

[Nicolas Dufresne]

Hi,

Le samedi 27 juin 2026 à 08:39 +0200, Doruk Tan Ozturk a écrit :
> codec_vp9_rm_noshow_frame() frees the first non-shown reference frame on
> ref_frames_list without excluding frames that are still in use. 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. Freeing a frame that
> is still an active reference (codec_vp9_is_ref()) or the current frame
> has the same in-use-then-free shape and additionally desyncs the
> reference bookkeeping in codec_vp9_sync_ref().
> 
> The sibling cleanup codec_vp9_show_frame() already guards exactly these
> cases before freeing:
> 
> 	if (codec_vp9_is_ref(vp9, tmp) || tmp == vp9->prev_frame)
> 		continue;
> 
> rm_noshow_frame() simply omits the same check. Add it, also skipping
> cur_frame, so both cleanup paths agree on which frames are safe to free.
> 
> 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 0sec's autonomous vulnerability analysis (https://0sec.ai).
> Found by static analysis; not yet runtime-reproduced (Amlogic Meson
> hardware required).
> 
> Fixes: 00c43088aa68 ("media: meson: vdec: add VP9 decoder support")
> Signed-off-by: Doruk Tan Ozturk <doruk@xxxxxxx>

I can't really do anything about this patch without an actual Tested-by. Perhaps
try and ping someone with the hardware to let this patch progress, meanwhile
I'll pass.

This is also a staging driver that is likely to be removed in few cycles
considering it not being maintain and there is several competing drivers being
developed.

Nicolas

> ---
> v2: Per Dan Carpenter's review, also skip active reference frames
>     (codec_vp9_is_ref()) and cur_frame, matching codec_vp9_show_frame()
>     exactly — freeing an in-use altref/reference frame here also caused a
>     codec_vp9_sync_ref() desync, not just the prev_frame UAF.
> 
>  drivers/staging/media/meson/vdec/codec_vp9.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/staging/media/meson/vdec/codec_vp9.c b/drivers/staging/media/meson/vdec/codec_vp9.c
> index 8e80ecf84193..dad75950933c 100644
> --- a/drivers/staging/media/meson/vdec/codec_vp9.c
> +++ b/drivers/staging/media/meson/vdec/codec_vp9.c
> @@ -1238,6 +1238,8 @@ static void codec_vp9_show_existing_frame(struct codec_vp9 *vp9)
>  	pr_debug("showing frame %u\n", param->p.frame_to_show_idx);
>  }
>  
> +static bool codec_vp9_is_ref(struct codec_vp9 *vp9, struct vp9_frame *frame);
> +
>  static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
>  {
>  	struct codec_vp9 *vp9 = sess->priv;
> @@ -1247,6 +1249,18 @@ static void codec_vp9_rm_noshow_frame(struct amvdec_session *sess)
>  		if (tmp->show)
>  			continue;
>  
> +		/*
> +		 * Mirror codec_vp9_show_frame(): never free an active
> +		 * reference frame, the previously decoded frame, or the
> +		 * current frame here. prev_frame is still dereferenced by the
> +		 * MV predictor in codec_vp9_set_mpred_mv(), and freeing an
> +		 * in-use altref/reference also desyncs codec_vp9_sync_ref();
> +		 * either is a use-after-free of an in-use frame.
> +		 */
> +		if (codec_vp9_is_ref(vp9, tmp) || tmp == vp9->prev_frame ||
> +		    tmp == vp9->cur_frame)
> +			continue;
> +
>  		pr_debug("rm noshow: %u\n", tmp->index);
>  		v4l2_m2m_buf_queue(sess->m2m_ctx, tmp->vbuf);
>  		list_del(&tmp->list);

Attachment: signature.asc
Description: This is a digitally signed message part