Re: [PATCH] media: meson: vdec: fix NULL pointer deref in vdec_try_fmt_common

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

Pre-existing issues:
- [High] Missing V4L2 queue busy check in vdec_s_fmt allows format changes during active streaming, leading to type confusion and memory corruption in the ISR.
--

commit 8aa33adb9b745ad7b1a26721ccafdbeb5f0768bb
Author: Christian Hewitt <[email protected]>

media: meson: vdec: fix NULL pointer deref in vdec_try_fmt_common

This commit fixes a NULL pointer dereference in vdec_try_fmt_common() by
falling back to the first format in the platform's format array instead of
hardcoding MPEG2. This prevents crashes on systems where MPEG2 support has
been locally patched out.

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index a039d925c0fe5..c18bf352b1996 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but while looking at the format handling in
this driver, I noticed a potential race condition.

Does vdec_s_fmt() allow format changes during active streaming?

drivers/staging/media/meson/vdec/vdec.c:vdec_s_fmt() {
        ...
	if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
		sess->fmt_out = fmt_out;
	else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
		sess->pixfmt_cap = format.fmt.pix_mp.pixelformat;
        ...
}

It appears there is no check to ensure the V4L2 queues are idle (e.g.,
via vb2_is_busy()) before overwriting the session format parameters.

If a userspace application is actively streaming and concurrently calls
VIDIOC_S_FMT with a different format, could this lead to type confusion
and memory corruption in the hardirq handler?

drivers/staging/media/meson/vdec/vdec.c:vdec_isr() {
        ...
	return sess->fmt_out->codec_ops->isr(sess);
}

Because the hardirq handler dereferences sess->fmt_out without locks,
a mid-stream format change might cause the ISR of the newly selected codec
to execute on a session state (sess->priv) initialized for the previous
codec.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic