[PATCH v7 18/19] media: meson: vdec: Increase VIFIFO buffer size to 32 MiB

[sashiko-bot at kernel.org]

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

New issues:
- [High] The `esparser_queue_all_src` workqueue handler contains a fatal infinite loop due to incorrect usage of the `scoped_guard` macro in combination with `break` statements, monopolizing 100% of a CPU core.
- [High] Double decrement of the `esparser_queued_bufs` atomic counter per video frame leads to counter underflow and buffer overruns.
- [High] The 32 MiB VIFIFO buffer increase causes the bitstream size to overflow the 32-bit hardware PARSER_CONTROL register calculation, crashing the esparser DMA engine.

Pre-existing issues:
- [Critical] In `vp9_update_header`, an integer overflow when decoding maliciously crafted frame magnitudes allows `frame_size` to become negative, bypassing size validations and triggering a massive out-of-bounds `memmove` that panics the kernel.
- [High] Physical address truncation for the hardware ring buffer end pointer causes immediate DMA corruption when memory is allocated near the top of the 32-bit address space.
- [High] Use-After-Free (UAF) vulnerabilities in `vdec_close` during device unbind due to `devm_kzalloc` and `v4l2_m2m_release` lifecycle mismatches.
- [Medium] Timestamp tracking and matching breaks silently when the 32-bit hardware offset wraps around at the 4 GB bitstream boundary, due to an improper naive unsigned sequence comparison.
- [Medium] Race condition in `esparser_write_data` overwrites the completion flag, causing false input parsing timeouts.
--

--- Patch [18]: [PATCH v7 18/19] media: meson: vdec: Increase VIFIFO buffer size to 32 MiB ---
commit 2addc7a4961dc5fcc69dfbb2e96d57eb89853de9
Author: Anand Moon <linux.amoon at gmail.com>

media: meson: vdec: Increase VIFIFO buffer size to 32 MiB

During 4K video playback, the hardware Elementary Stream Parser (esparser)
can rapidly push massive burst data payloads into the Video Input FIFO
faster than the VPU decoding block can consume it. This causes a buffer
overflow when stream usage breaks past the allocated hardware boundary.
This patch doubles the parsed bitstream swap exchange memory pool to 32 MiB.

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index d33cbebc4453b..824e2f156adeb 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -29,8 +29,8 @@ struct dummy_buf {
>  	struct list_head list;
>  };
>  
> -/* 16 MiB for parsed bitstream swap exchange */
> -#define SIZE_VIFIFO SZ_16M
> +/* 32 MiB for parsed bitstream swap exchange */
> +#define SIZE_VIFIFO SZ_32M

[Severity: High]
Does increasing the buffer to 32 MiB cause the bitstream size to overflow
the 32-bit hardware PARSER_CONTROL register calculation?

In drivers/staging/media/meson/vdec/esparser.c:esparser_write_data():

	amvdec_write_parser(core, PARSER_CONTROL,
			    ES_WRITE |
			    ES_PARSER_START |
			    ES_SEARCH |
			    (size << ES_PACK_SIZE_BIT));

With ES_PACK_SIZE_BIT being 8, for a size >= 16 MiB this left shift will
overflow a 32-bit unsigned integer. This can result in the DMA engine
receiving a truncated payload size and crashing.

[Severity: High]
This is a pre-existing issue, but does increasing the buffer size make it
highly probable that the hardware ring buffer end pointer truncates, causing
immediate DMA corruption?

In drivers/staging/media/meson/vdec/vdec_hevc.c:vdec_hevc_stbuf_init():

	amvdec_write_dos(core, HEVC_STREAM_END_ADDR,
			 sess->vififo_paddr + sess->vififo_size);

If the VIFIFO buffer is allocated near the end of the 32-bit address space,
the calculation sess->vififo_paddr + sess->vififo_size can exceed 32 bits,
resulting in a value like 0x100000000. This is silently truncated to 0 when
passed to amvdec_write_dos() which accepts a 32-bit u32.

[Severity: Medium]
This is a pre-existing issue, but how does the driver handle timestamps when
the 32-bit hardware offset wraps around at the 4 GB bitstream boundary?

In drivers/staging/media/meson/vdec/vdec_helpers.c:amvdec_dst_buf_done_offset():

	list_for_each_entry_safe(tmp, n, &sess->timestamps, list) {
		if (tmp->offset > offset) {
			...
			break;
		}

The stream offset accumulates into a 32-bit integer in esparser_get_offset()
that naturally wraps at 4 GB. The list_for_each_entry_safe loop uses a
non-wrap-safe raw unsigned comparison. When the offset wraps around to 0,
a large tmp->offset immediately evaluates as greater and incorrectly breaks
the loop, stranding or dropping timestamps.

[Severity: High]
Are there Use-After-Free vulnerabilities in vdec_close() during device unbind
due to devm_kzalloc() and v4l2_m2m_release() lifecycle mismatches? This isn't
a bug introduced by this patch, but...

In drivers/staging/media/meson/vdec/vdec.c:vdec_remove():

	video_unregister_device(core->vdev_dec);
	v4l2_m2m_release(core->m2m_dev);

The unbind path frees the core structure while V4L2 file descriptors may
still be actively held by userspace.

Later, if an application closes the file descriptor, vdec_close() is invoked:

	synchronize_irq(core->vdec_irq);

This could dereference the freed core pointer and the freed m2m_dev.

[Severity: High]
Does the esparser_queue_all_src() workqueue handler contain an infinite loop
that could monopolize a CPU core?

In drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src():

	while (1) {
		struct vb2_v4l2_buffer *vbuf = NULL;
		bool processed_frame = false;

		scoped_guard(mutex, &sess->lock) {
			...
			vbuf = v4l2_m2m_next_src_buf(sess->m2m_ctx);
			if (!vbuf)
				break;
			...
		}

		if (processed_frame)
			break;
		...
	}

The break statements are intended to exit the function when no buffers are
available. However, they only escape the scoped_guard's internal for loop.
This leaves the outer while (1) loop intact since processed_frame is false,
which could result in a lockup of a kernel worker thread.

[Severity: High]
Does this code double decrement the esparser_queued_bufs atomic counter?

In drivers/staging/media/meson/vdec/vdec_helpers.c, dst_buf_done() calls
atomic_dec(&sess->esparser_queued_bufs). However, its callers also execute
their own atomic_dec():

amvdec_dst_buf_done():

	dst_buf_done(sess, vbuf, field, timestamp, timecode, vbuf_flags);
	atomic_dec(&sess->esparser_queued_bufs);

amvdec_dst_buf_done_offset():

	dst_buf_done(sess, vbuf, field, timestamp, timecode, vbuf_flags);
	if (match)
		atomic_dec(&sess->esparser_queued_bufs);

This could lead to counter underflow, bypassing buffer limit checks and
causing hardware queue overruns.

[Severity: Medium]
This is a pre-existing issue, but does esparser_write_data() contain a race
condition that overwrites the completion flag?

In drivers/staging/media/meson/vdec/esparser.c:esparser_write_data():

	amvdec_write_parser(core, PARSER_FETCH_CMD,
			    (7 << FETCH_ENDIAN_BIT) |
			    (size + SEARCH_PATTERN_LEN));

	search_done = 0;
	return wait_event_interruptible_timeout(wq, search_done, (HZ / 5));

The CPU initiates hardware processing by writing to PARSER_FETCH_CMD, which
can instantly trigger esparser_isr(). If the hardware interrupt fires
immediately, the ISR sets search_done = 1. The CPU sequentially executes
search_done = 0 afterwards, which could erase the completion signal before
waiting on it.

[Severity: Critical]
This is a pre-existing issue, but does vp9_update_header() have an integer
overflow when decoding frame magnitudes that can bypass size validations?

In drivers/staging/media/meson/vdec/esparser.c:vp9_update_header():

				frame_size[cur_frame] |=
					(dp[mag_ptr] << (cur_mag * 8));

The magnitude byte is promoted to a signed 32-bit integer. When shifted by
24 bits, it can become a large negative number if the maliciously crafted
payload has dp[mag_ptr] >= 0x80.

Later, this negative frame size bypasses total_datasize validations and
triggers an out-of-bounds copy in memmove:

		memmove(fdata + VP9_HEADER_SIZE, old_framedata, framesize);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713120840.17427-1-linux.amoon@gmail.com?part=18