[PATCH] media: meson: vdec: size capture planes from the aligned canvas

[PATCH] media: meson: vdec: size capture planes from the aligned canvas

[Michael Freidkin]

The decoder writes a whole canvas of ALIGN(width, 32) x ALIGN(height, 32)
(amvdec_set_canvases()) and reports bytesperline = ALIGN(width, 32), but
get_output_size() derives sizeimage from the raw width x height.

When the width is not a multiple of 32 the planes are too small for the
advertised stride: for 720x360 NV12M the luma plane is 262144 bytes while
736 * 360 = 264960 are needed (282624 for the canvas the firmware fills).
The decoder writes past the buffer, and importing the capture dma-buf
into DRM fails: drmModeAddFB2() returns -EINVAL, so e.g. Kodi plays the
sound over a black screen. 1280x720 and 1920x1080 are not affected.

Size the planes from the aligned canvas the hardware actually uses.

Tested on an S905X (GXL p212) board with LibreELEC 12 (6.16.0-rc3):
720x360 and 1920x1080 H.264 play through V4L2 m2m + DRM PRIME with no
AddFB2 errors.

Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Michael Freidkin <freidkin@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -34,7 +34,7 @@
 
 static u32 get_output_size(u32 width, u32 height)
 {
-	return ALIGN(width * height, SZ_64K);
+	return ALIGN(ALIGN(width, 32) * ALIGN(height, 32), SZ_64K);
 }
 
 u32 amvdec_get_output_size(struct amvdec_session *sess)
--

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

Re: [PATCH] media: meson: vdec: size capture planes from the aligned canvas

[Dan Carpenter]

On Mon, Sep 28, 2026 at 07:50:45PM +0300, Michael Freidkin wrote:
> The decoder writes a whole canvas of ALIGN(width, 32) x ALIGN(height, 32)
> (amvdec_set_canvases()) and reports bytesperline = ALIGN(width, 32), but
> get_output_size() derives sizeimage from the raw width x height.
> 
> When the width is not a multiple of 32 the planes are too small for the
> advertised stride: for 720x360 NV12M the luma plane is 262144 bytes while
> 736 * 360 = 264960 are needed (282624 for the canvas the firmware fills).
> The decoder writes past the buffer, and importing the capture dma-buf
> into DRM fails: drmModeAddFB2() returns -EINVAL, so e.g. Kodi plays the
> sound over a black screen. 1280x720 and 1920x1080 are not affected.
> 
> Size the planes from the aligned canvas the hardware actually uses.
> 
> Tested on an S905X (GXL p212) board with LibreELEC 12 (6.16.0-rc3):
> 720x360 and 1920x1080 H.264 play through V4L2 m2m + DRM PRIME with no
> AddFB2 errors.
> 
> Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
> Signed-off-by: Michael Freidkin <freidkin@gmail.com>

Reviewed-by: Dan Carpenter <error27@gmail.com>

It would probably be more reliable to just ALIGN() the width and
height at the start so we don't have to do it over and over again.

regards,
dan carpenter


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

Re: [PATCH] media: meson: vdec: size capture planes from the aligned canvas

[Michael Freidkin]

On Tue, Sep 29, 2026 at 08:46:45AM +0300, Dan Carpenter wrote:
> Reviewed-by: Dan Carpenter <error27@gmail.com>
>
> It would probably be more reliable to just ALIGN() the width and
> height at the start so we don't have to do it over and over again.

Thanks for the review, Dan.

Agreed. I kept this one minimal so it backports cleanly with the Fixes
tag. If the maintainers prefer, I can send a v2 that aligns width and
height once in vdec_try_fmt_common() and derives bytesperline and the
plane sizes from them, or do that as a follow-up cleanup on top.

Michael

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

Re: [PATCH] media: meson: vdec: size capture planes from the aligned canvas

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] media: meson: vdec: integer overflow in get_output_size() causes OOB DMA write
--

commit 28bcd6a9db6c6247ecace49156692588db867097
Author: Michael Freidkin <freidkin@gmail.com>

media: meson: vdec: size capture planes from the aligned canvas

This commit aligns the width and height to 32 before calculating the output
size in get_output_size(). This fixes an issue where the allocated capture
planes were too small for the advertised stride when the width is not a
multiple of 32.

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index c18bf352b1996..be29d5a11c2e6 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -34,7 +34,7 @@ struct dummy_buf {
>  
>  static u32 get_output_size(u32 width, u32 height)
>  {
> -	return ALIGN(width * height, SZ_64K);
> +	return ALIGN(ALIGN(width, 32) * ALIGN(height, 32), SZ_64K);

[Severity: Critical]
Does this new alignment calculation introduce an integer overflow that
results in a 0-byte DMA buffer?

A crafted VP9 bitstream can provide untrusted dimensions up to 65535x65535.
The firmware parses these into param->p.width and param->p.height in
drivers/staging/media/meson/vdec/codec_vp9.c:codec_vp9_process_rpm(), which
passes them directly to amvdec_src_change(). The dimensions are assigned
to sess->width and sess->height without clamping.

If a user handles the resulting source change event by calling
VIDIOC_REQBUFS directly, skipping VIDIOC_S_FMT which would normally clamp
the dimensions, get_output_size() is called with 65535x65535.

When these dimensions are aligned to 32 here, ALIGN(65535, 32) *
ALIGN(65535, 32) evaluates to 65536 * 65536, or 4,294,967,296. Because
get_output_size() uses u32, this multiplication overflows and wraps
exactly to 0.

vdec_queue_setup() then fetches this overflowed output size (0) to
initialize the DMA plane sizes, bypassing size validations and requesting a
0-byte DMA buffer. The hardware would then attempt to decode the frame into
this tiny buffer, writing gigabytes of data out of bounds.

Prior to this patch, width * height for 65535x65535 was 4,294,836,225,
which did not overflow u32 and correctly caused a safe allocation failure.

>  }
>  
>  u32 amvdec_get_output_size(struct amvdec_session *sess)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928165045.642354-1-freidkin@gmail.com?part=1

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