Re: [PATCH 2/2] media: meson: vdec: guard against NULL prev_frame in codec_vp9_set_mpred_mv()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The patch introduces a critical vulnerability by using an early return to fix a NULL pointer dereference. When `vp9->prev_frame` is NULL, `codec_vp9_set_mpred_mv()` returns early and skips programming the hardware's motion vector write registers (`HEVC_MPRED_MV_WR_START_ADDR` and `HEVC_MPRED_MV_WPTR`) for the current frame. When the decoder is subsequently started, it performs a wild DMA write to whatever uninitialized or stale physical address is lingering in the hardware register, causing arbitrary physical memory corruption.
--

commit bf11770a8f9c71282f324df9d5ac30ccdcb86996
Author: Doruk Tan Ozturk <doruk@0sec.ai>
media: meson: vdec: guard against NULL prev_frame in codec_vp9_set_mpred_mv()

This commit addresses a NULL pointer dereference in codec_vp9_set_mpred_mv()
by adding an early return when vp9->prev_frame is NULL. The crash would
previously trigger when decoding malformed bitstreams that start on a non-key
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
[ ... ]
> @@ -993,19 +993,32 @@ static void codec_vp9_set_mpred_mv(struct amvdec_core *core,
[ ... ]
> +	/*
> +	 * prev_frame is NULL when an inter frame is the first frame decoded
> +	 * (e.g. a stream starting on a non-key frame, or the first frame
> +	 * after a flush). There are no previous-frame motion vectors to use
> +	 * and every read below would dereference a NULL pointer, so leave
> +	 * prev-MV use disabled (BIT(6) already cleared) and bail out.
> +	 */
> +	if (!vp9->prev_frame)
> +		return;

[Severity: Critical]
Does this early return bypass the initialization of the hardware's motion
vector write registers for the current frame?

Looking at the rest of codec_vp9_set_mpred_mv(), the following registers are
set after this early return block:

	amvdec_write_dos(core, HEVC_MPRED_MV_WR_START_ADDR,
			 codec_vp9_get_frame_mv_paddr(vp9, vp9->cur_frame));
	amvdec_write_dos(core, HEVC_MPRED_MV_WPTR,
			 codec_vp9_get_frame_mv_paddr(vp9, vp9->cur_frame));

If these writes are skipped when vp9->prev_frame is NULL, could the decoder
perform unconstrained DMA writes to whatever uninitialized or stale physical
addresses are lingering in the hardware register when decoding begins?

Since this state can be reached via malformed streams from unprivileged users
as noted in the commit message, can this lead to arbitrary physical memory
corruption?

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

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