Re: [PATCH v1 1/3] media: sun6i-csi: capture: Align size to 16 for NV12_16L16

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: sun6i-csi: incorrect bytesperline for NV12_16L16 format
--

commit 476de016f26cd243aac004fdf752c97c70ae693b
Author: Arash Golgol <arash.golgol@gmail.com>

    media: sun6i-csi: capture: Align size to 16 for NV12_16L16

This commit introduces alignment handling specific to pixel formats in the
sun6i-csi capture driver. It sets the required alignment based on the format
and applies it during format preparation.

> 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..f55edfd0b174a 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c

[ ... ]

> @@ -665,17 +671,19 @@ static void sun6i_csi_capture_format_prepare(struct v4l2_format *format)
>  {
>  	struct v4l2_pix_format *pix_format = &format->fmt.pix;
>  	const struct v4l2_format_info *info;
> -	unsigned int width, height;
> -
> -	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);
> +	unsigned int width, height, align;
>  
>  	if (!sun6i_csi_capture_format_find(pix_format->pixelformat))
>  		pix_format->pixelformat =
>  			sun6i_csi_capture_formats[0].pixelformat;
>  
> +	align = sun6i_csi_capture_format_align(pix_format->pixelformat);
> +
> +	v4l_bound_align_image(&pix_format->width, SUN6I_CSI_CAPTURE_WIDTH_MIN,
> +			      SUN6I_CSI_CAPTURE_WIDTH_MAX, align,
> +			      &pix_format->height, SUN6I_CSI_CAPTURE_HEIGHT_MIN,
> +			      SUN6I_CSI_CAPTURE_HEIGHT_MAX, align, 0);
> +
>  	width = pix_format->width;
>  	height = pix_format->height;
>
> 	info = v4l2_format_info(pix_format->pixelformat);
>
> 	switch (pix_format->pixelformat) {
> 	case V4L2_PIX_FMT_NV12_16L16:
> 		pix_format->bytesperline = width * 12 / 8;

[Severity: Medium]
Does this code violate the V4L2 API contract by incorrectly hardcoding
bytesperline to 1.5x width as a shortcut to calculate the correct sizeimage?

The V4L2 specification requires bytesperline to reflect the stride of the
largest plane, which for NV12 is just width. By exposing a stride of
1.5 * width, userspace applications may compute out-of-bounds plane offsets,
potentially leading to memory corruption or crashes within the userspace
client.

This is supported by sun6i_csi_setup_format(), which configures the
hardware using the correct luma and chroma strides of width internally:

drivers/media/platform/sunxi/sun6i-csi/sun6i_csi_capture.c:sun6i_csi_setup_format() {
	...
	switch (pixelformat) {
	...
	case V4L2_PIX_FMT_NV12_16L16:
		luma_line = width;
		chroma_line = width;
		break;
	...
}

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