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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `vidioc_enum_framesizes` incorrectly advertises a 2-pixel step size for `V4L2_PIX_FMT_NV12_16L16`, violating the format's strict 16-pixel alignment requirement.
--

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

media: sun6i-csi: capture: Implement vidioc_enum_framesizes

This commit implements vidioc_enum_framesizes to report the stepwise
frame size range supported by the CSI capture hardware. It sets the
supported dimensions with a uniform step width and height of 2.

> 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..f788b42346739 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,27 @@ 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;
> +	fsize->stepwise.step_width = 2;
> +	fsize->stepwise.step_height = 2;

[Severity: High]
Does this hardcoded step size properly handle V4L2_PIX_FMT_NV12_16L16?

The V4L2 specification requires both the line stride and image height for
V4L2_PIX_FMT_NV12_16L16 to be a multiple of 16, as it is a 16x16 tiled
format. By setting step_width and step_height to 2 for all formats, this
advertises that arbitrary even dimensions are valid for the tiled format.

If userspace requests a non-16-aligned dimension, such as 18x18, the driver
may calculate an undersized buffer. Because the hardware operates on
complete 16x16 macroblocks, it will attempt to DMA at least a 32x32 block to
cover the 18x18 image, resulting in an out-of-bounds DMA memory write.

Should the step sizes be dynamically adjusted based on fsize->pixel_format
to enforce 16-pixel block alignment when necessary?

> +
> +	return 0;
> +}
> +

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