Re: [PATCH v2 02/11] staging: media: cedrus: reject too large H.264/HEVC SPS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] media: cedrus: missing lower-bound check in
  cedrus_check_pic_size()
--

commit 2d66c4366bc66c197579905e0bf065bb7fd5088c
Author: Jernej Skrabec <jernej.skrabec@gmail.com>

staging: media: cedrus: reject too large H.264/HEVC SPS

This commit checks H.264 and HEVC SPS picture dimensions against the
maximum hardware limits in cedrus_try_ctrl() and against the aligned
source format dimensions in the codec setup callbacks.

> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 54f867949fbae..c2318521d7e5a 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -274,6 +274,40 @@ cedrus_is_capable(struct cedrus_ctx *ctx, unsigned int capabilities)
>  	return (ctx->dev->capabilities & capabilities) == capabilities;
>  }
>  
> +/*
> + * The decoders take the picture size, or the position of the slices, from
> + * the bitstream headers. Check that the picture fits the buffers, which are
> + * sized from the OUTPUT format and padded to at least 16 pixels in both
> + * directions, so that the hardware doesn't overrun them and hang.
> + */
> +static inline int cedrus_check_pic_size(struct cedrus_ctx *ctx,
> +					unsigned int width, unsigned int height)
> +{
> +	if (width > ALIGN(ctx->src_fmt.width, 16) ||
> +	    height > ALIGN(ctx->src_fmt.height, 16))
> +		return -EINVAL;
> +
> +	return 0;
> +}

[Severity: High]
Should cedrus_check_pic_size() and cedrus_pic_size_supported() also
reject zero dimensions or sizes below the 16x16 minimum?

Unlike H.264 where cedrus_h264_sps_width() and cedrus_h264_sps_height()
are always at least 16, v4l2_ctrl_hevc_sps stores raw sample counts in
pic_width_in_luma_samples and pic_height_in_luma_samples.  Those fields
default to 0 in std_init_compound() and are not rejected when 0 by
std_validate_compound().  If userspace submits an HEVC decode request
without setting the SPS control, or sets pic_width_in_luma_samples to 0,
both checks pass and cedrus_h265_setup() divides by zero when computing
ctb_addr_x and ctb_addr_y:

drivers/staging/media/sunxi/cedrus/cedrus_h265.c:cedrus_h265_setup() {
    ...
	width_in_ctb_luma =
		DIV_ROUND_UP(sps->pic_width_in_luma_samples, ctb_size_luma);
    ...
	ctb_addr_x = slice_params->slice_segment_addr % width_in_ctb_luma;
	ctb_addr_y = slice_params->slice_segment_addr / width_in_ctb_luma;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109175016.42061-1-jernej.skrabec@gmail.com?part=2