media: cedrus: validate H.264 reference list indices

Message ID 20260324020431.1800-1-pengpeng@iscas.ac.cn (mailing list archive)
State New
Headers
Series media: cedrus: validate H.264 reference list indices |

Commit Message

Pengpeng Hou March 24, 2026, 2:04 a.m. UTC
Cedrus validates HEVC slice reference lists in cedrus_try_ctrl(), but
the H.264 path still consumes ref_pic_list0/ref_pic_list1 indices
straight from the stateless slice control. Those indices are later
used to index the fixed-size decode_params->dpb[] array in
_cedrus_write_ref_list().

Reject H.264 slice controls whose active reference counts or
reference indices exceed V4L2_H264_NUM_DPB_ENTRIES before the driver
reaches the DPB lookup. This keeps the validation next to the existing
Cedrus stateless control checks and avoids driver-specific
out-of-bounds reads from malformed userspace control payloads.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 23 +++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

Comments

Jernej Skrabec March 24, 2026, 7:56 a.m. UTC | #1
CC: Nicolas

Dne torek, 24. marec 2026 ob 03:04:31 Srednjeevropski standardni čas je Pengpeng Hou napisal(a):
> Cedrus validates HEVC slice reference lists in cedrus_try_ctrl(), but
> the H.264 path still consumes ref_pic_list0/ref_pic_list1 indices
> straight from the stateless slice control. Those indices are later
> used to index the fixed-size decode_params->dpb[] array in
> _cedrus_write_ref_list().
> 
> Reject H.264 slice controls whose active reference counts or
> reference indices exceed V4L2_H264_NUM_DPB_ENTRIES before the driver
> reaches the DPB lookup. This keeps the validation next to the existing
> Cedrus stateless control checks and avoids driver-specific
> out-of-bounds reads from malformed userspace control payloads.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

This has same issue as doing it in common code, e.g. it would break
userspace.

One improvement would be to skip all indices which have value higher
or equal to V4L2_H264_NUM_DPB_ENTRIES here:

https://elixir.bootlin.com/linux/v6.19.9/source/drivers/staging/media/sunxi/cedrus/cedrus_h264.c#L212

Best regards,
Jernej

> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c | 23 +++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index d68da1eaa7aa..905084c097a9 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -42,6 +42,29 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  		if (sps->bit_depth_luma_minus8 != 0)
>  			/* Only 8-bit is supported */
>  			return -EINVAL;
> +	} else if (ctrl->id == V4L2_CID_STATELESS_H264_SLICE_PARAMS) {
> +		const struct v4l2_ctrl_h264_slice_params *slice = ctrl->p_new.p_h264_slice_params;
> +		unsigned int i;
> +
> +		if (slice->num_ref_idx_l0_active_minus1 >=
> +		    V4L2_H264_NUM_DPB_ENTRIES)
> +			return -EINVAL;
> +
> +		for (i = 0; i <= slice->num_ref_idx_l0_active_minus1; i++)
> +			if (slice->ref_pic_list0[i].index >=
> +			    V4L2_H264_NUM_DPB_ENTRIES)
> +				return -EINVAL;
> +
> +		if (slice->slice_type == V4L2_H264_SLICE_TYPE_B) {
> +			if (slice->num_ref_idx_l1_active_minus1 >=
> +			    V4L2_H264_NUM_DPB_ENTRIES)
> +				return -EINVAL;
> +
> +			for (i = 0; i <= slice->num_ref_idx_l1_active_minus1; i++)
> +				if (slice->ref_pic_list1[i].index >=
> +				    V4L2_H264_NUM_DPB_ENTRIES)
> +					return -EINVAL;
> +		}
>  	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
>  		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
>  		struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);
>
  
Pengpeng Hou March 24, 2026, 8:08 a.m. UTC | #2
Hi Jernej,

Thanks, that makes sense.

You're right that rejecting the H.264 slice control in cedrus_try_ctrl()
would have the same userspace compatibility problem as the HEVC case.
I'll drop that approach and respin this as a separate Cedrus patch that
skips out-of-range reference list indices at the point where
_cedrus_write_ref_list() resolves them into decode->dpb[] entries.

Best regards,
Pengpeng
  

Patch

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index d68da1eaa7aa..905084c097a9 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -42,6 +42,29 @@  static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 		if (sps->bit_depth_luma_minus8 != 0)
 			/* Only 8-bit is supported */
 			return -EINVAL;
+	} else if (ctrl->id == V4L2_CID_STATELESS_H264_SLICE_PARAMS) {
+		const struct v4l2_ctrl_h264_slice_params *slice = ctrl->p_new.p_h264_slice_params;
+		unsigned int i;
+
+		if (slice->num_ref_idx_l0_active_minus1 >=
+		    V4L2_H264_NUM_DPB_ENTRIES)
+			return -EINVAL;
+
+		for (i = 0; i <= slice->num_ref_idx_l0_active_minus1; i++)
+			if (slice->ref_pic_list0[i].index >=
+			    V4L2_H264_NUM_DPB_ENTRIES)
+				return -EINVAL;
+
+		if (slice->slice_type == V4L2_H264_SLICE_TYPE_B) {
+			if (slice->num_ref_idx_l1_active_minus1 >=
+			    V4L2_H264_NUM_DPB_ENTRIES)
+				return -EINVAL;
+
+			for (i = 0; i <= slice->num_ref_idx_l1_active_minus1; i++)
+				if (slice->ref_pic_list1[i].index >=
+				    V4L2_H264_NUM_DPB_ENTRIES)
+					return -EINVAL;
+		}
 	} else if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
 		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
 		struct cedrus_ctx *ctx = container_of(ctrl->handler, struct cedrus_ctx, hdl);