Re: [PATCH v5 2/2] media: sun6i-csi: capture: Implement vidioc_enum_framesizes

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Inconsistent frame size constraints between `VIDIOC_ENUM_FRAMESIZES` and `VIDIOC_TRY_FMT` / `VIDIOC_S_FMT` for `V4L2_PIX_FMT_NV12_16L16`.

Pre-existing issues:
- [High] `TRY_FMT` calculates `sizeimage` incorrectly for `NV12_16L16` with unaligned dimensions, leading to potential DMA buffer overflows.
--

--- Patch [2]: [PATCH v5 2/2] media: sun6i-csi: capture: Implement vidioc_enum_framesizes ---
commit f5cc5843b18ce4fd2988ebb6c61247d1f5cd63a3
Author: Arash Golgol <arash.golgol@gmail.com>
media: sun6i-csi: capture: Implement vidioc_enum_framesizes

This commit reports the stepwise frame size range supported by the CSI
capture hardware for the pixel formats exposed by the driver.

> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> index d90abba213098..e17c7f82ff478 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> @@ -738,6 +738,33 @@ static int sun6i_csi_capture_enum_fmt(struct file *file, void *priv,
>  	return 0;
>  }
>  
> +static int sun6i_csi_capture_enum_framesize(struct file *file, void *fh,
> +					    struct v4l2_frmsizeenum *fsize)
> +{
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	/* Only accept format in map table. */
> +	if (!sun6i_csi_capture_format_find(fsize->pixel_format))
> +		return -EINVAL;
> +
> +	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
> +	fsize->stepwise.min_width = SUN6I_CSI_CAPTURE_WIDTH_MIN;
> +	fsize->stepwise.max_width = SUN6I_CSI_CAPTURE_WIDTH_MAX;
> +	fsize->stepwise.min_height = SUN6I_CSI_CAPTURE_HEIGHT_MIN;
> +	fsize->stepwise.max_height = SUN6I_CSI_CAPTURE_HEIGHT_MAX;
> +
> +	if (fsize->pixel_format == V4L2_PIX_FMT_NV12_16L16) {
> +		fsize->stepwise.step_width = 16;
> +		fsize->stepwise.step_height = 16;
> +	} else {
> +		fsize->stepwise.step_width = 2;
> +		fsize->stepwise.step_height = 2;
> +	}

[Severity: Medium]
Will this lead to inconsistent frame size constraints?

While this reports a 16-pixel step width and height for
V4L2_PIX_FMT_NV12_16L16 via ENUM_FRAMESIZES,
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c:sun6i_csi_capture_format_prepare()
(used by TRY_FMT and S_FMT) unconditionally uses v4l_bound_align_image()
with an alignment of 2:

	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
			      SUN6I_CSI_CAPTURE_WIDTH_MAX, 1,
			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, 1, 0);

Could this allow userspace to successfully request sizes via TRY_FMT that
ENUM_FRAMESIZES claims are unsupported, potentially leading to
v4l2-compliance test failures and contradictory format negotiation?

[Severity: High]
This isn't a bug introduced by this patch, but does TRY_FMT calculate
sizeimage incorrectly for NV12_16L16 with unaligned dimensions?

Since
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c:sun6i_csi_capture_format_prepare()
allows width and height to be unaligned multiples of 2 and computes
sizeimage purely linearly:

	case V4L2_PIX_FMT_NV12_16L16:
		pix_format->bytesperline = width * 12 / 8;
		pix_format->sizeimage = pix_format->bytesperline * height;
		break;

Could this lead to a DMA buffer overflow?

The CSI DMA hardware natively writes in 16x16 tiles. If a user requests an
unaligned frame size (e.g., 34x34) for V4L2_PIX_FMT_NV12_16L16 via
VIDIOC_TRY_FMT, the allocated vb2 buffer will be smaller than what the
hardware DMA actually writes.

For example, a 34x34 image will be written as 3x3 tiles (48x48 pixels),
consuming 3456 bytes. However, the allocated buffer for 34x34 is only 1734
bytes (34 * 1.5 * 34), which would cause a buffer overflow when the DMA
writes the padded tiles past the end of the allocation.

> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719112714.72802-1-arash.golgol@gmail.com?part=2