Re: [PATCH] media: cedrus: skip invalid H.264 reference list entries

[Nicolas Dufresne]

Le mardi 24 mars 2026 à 16:08 +0800, Pengpeng Hou a écrit :
> Cedrus consumes H.264 ref_pic_list0/ref_pic_list1 entries from the
> stateless slice control and later uses their indices to look up
> decode->dpb[] in _cedrus_write_ref_list().
> 
> Rejecting such controls in cedrus_try_ctrl() would break existing
> userspace, since stateless H.264 reference lists may legitimately carry
> out-of-range indices for missing references. Instead, guard the actual
> DPB lookup in Cedrus and skip entries whose indices do not fit the fixed
> V4L2_H264_NUM_DPB_ENTRIES array.
> 
> This keeps the fix local to the driver use site and avoids out-of-bounds
> reads from malformed or unsupported reference list entries.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@xxxxxxxxxxx>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus_h264.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -210,6 +210,9 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
>  		u8 dpb_idx;
>  
>  		dpb_idx = ref_list[i].index;
> +		if (dpb_idx >= V4L2_H264_NUM_DPB_ENTRIES)
> +			continue;

Matches how we skip inactive references (in this diff, though most userspace
just don't pass them). Now, if I looked lower, we set a position for each
references. My understanding is that if no bits are set, it means "no position".
How much testing have you done to confirm the HW behaves properly ?

Despite this question, I think this is going to work better then doing memory
overrun:

	Reviewed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>

Nicolas

> +
>  		dpb = &decode->dpb[dpb_idx];
>  
>  		if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))

Attachment: signature.asc
Description: This is a digitally signed message part