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@iscas.ac.cn>
---
 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;
+
 		dpb = &decode->dpb[dpb_idx];
 
 		if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
-- 
2.50.1

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@iscas.ac.cn>
> ---
>  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@collabora.com>

Nicolas

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

Dne torek, 24. marec 2026 ob 09:08:56 Srednjeevropski poletni čas je Pengpeng Hou napisal(a):
> 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@iscas.ac.cn>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej

On Sun, Mar 29, 2026 at 5:21 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne torek, 24. marec 2026 ob 09:08:56 Srednjeevropski poletni čas je Pengpeng Hou napisal(a):
> > 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@iscas.ac.cn>
>
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Tested-by: Chen-Yu Tsai <wens@kernel.org>

This fixes a KASAN slab-use-after-free warning when running fluster H.264
tests.

Le dimanche 29 mars 2026 à 20:44 +0800, Chen-Yu Tsai a écrit :
> On Sun, Mar 29, 2026 at 5:21 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> > 
> > Dne torek, 24. marec 2026 ob 09:08:56 Srednjeevropski poletni čas je Pengpeng Hou napisal(a):
> > > 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@iscas.ac.cn>
> > 
> > Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> 
> Tested-by: Chen-Yu Tsai <wens@kernel.org>
> 
> This fixes a KASAN slab-use-after-free warning when running fluster H.264
> tests.

Ah, very good, can you cite which test caused that ? I didn't expect fluster to
cover cases with missing references. I think it will be handy for future
testing.

Nicolas

On Mon, Mar 30, 2026 at 11:55 PM Nicolas Dufresne
<nicolas.dufresne@collabora.com> wrote:
>
> Le dimanche 29 mars 2026 à 20:44 +0800, Chen-Yu Tsai a écrit :
> > On Sun, Mar 29, 2026 at 5:21 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> > >
> > > Dne torek, 24. marec 2026 ob 09:08:56 Srednjeevropski poletni čas je Pengpeng Hou napisal(a):
> > > > 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@iscas.ac.cn>
> > >
> > > Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> >
> > Tested-by: Chen-Yu Tsai <wens@kernel.org>
> >
> > This fixes a KASAN slab-use-after-free warning when running fluster H.264
> > tests.
>
> Ah, very good, can you cite which test caused that ? I didn't expect fluster to
> cover cases with missing references. I think it will be handy for future
> testing.

Looks like it is FM1_BT_B. And it only happens on the first run after reboot,
or KASAN just only reports it once.

BTW, this would be a lot easier to figure out if we could get fluster to
output a system timestamp for each decode run (at least in single job mode).

I had to hack in delays between each decode rune, and then look at `dmesg -w`
and switching back to the window that has fluster running once the warning
triggers.


ChenYu

Le mardi 31 mars 2026 à 00:45 +0800, Chen-Yu Tsai a écrit :
> On Mon, Mar 30, 2026 at 11:55 PM Nicolas Dufresne
> <nicolas.dufresne@collabora.com> wrote:
> > 
> > Le dimanche 29 mars 2026 à 20:44 +0800, Chen-Yu Tsai a écrit :
> > > On Sun, Mar 29, 2026 at 5:21 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> > > > 
> > > > Dne torek, 24. marec 2026 ob 09:08:56 Srednjeevropski poletni čas je Pengpeng Hou napisal(a):
> > > > > 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@iscas.ac.cn>
> > > > 
> > > > Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > 
> > > Tested-by: Chen-Yu Tsai <wens@kernel.org>
> > > 
> > > This fixes a KASAN slab-use-after-free warning when running fluster H.264
> > > tests.
> > 
> > Ah, very good, can you cite which test caused that ? I didn't expect fluster to
> > cover cases with missing references. I think it will be handy for future
> > testing.
> 
> Looks like it is FM1_BT_B. And it only happens on the first run after reboot,
> or KASAN just only reports it once.

Thanks, its one of the unsupported stream that we didn't find how to detect
ahead of time, and so we try to decode it.

> 
> BTW, this would be a lot easier to figure out if we could get fluster to
> output a system timestamp for each decode run (at least in single job mode).


Well, that's not magical, they have to trace the same timestamp. An example, the
kernel and gstreamer both uses their own uptime, which is of course not helping
it at all.

> 
> I had to hack in delays between each decode rune, and then look at `dmesg -w`
> and switching back to the window that has fluster running once the warning
> triggers.

If all you care is which streams caused what kernel trace, I think the least
amount of effort is to propose a patch against fluster to syslog the start of
tests. Your logger will aggregate. Note that its only going to work for single
job run since the kernel error trace don't give enough context to trace back the
error into the V4L2 FD and back to the owning process.

Nicolas

> 
> 
> ChenYu