* [PATCH v2] media: v4l2-ctrls: validate AV1 ref_frame_idx and primary_ref_frame
@ 2026-07-09 13:12 Doruk Tan Ozturk
2026-09-28 20:51 ` Nicolas Dufresne
0 siblings, 1 reply; 2+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-09 13:12 UTC (permalink / raw)
To: hverkuil, mchehab, nicolas.dufresne
Cc: sakari.ailus, laurent.pinchart, detlev.casanova, kees,
michael.bommarito, linux-media, linux-kernel, Doruk Tan Ozturk
The stateless AV1 frame control V4L2_CID_STATELESS_AV1_FRAME carries
ref_frame_idx[V4L2_AV1_REFS_PER_FRAME] (signed, -128..127) and
primary_ref_frame (__u8), both copied from userspace. validate_av1_frame()
already checks flags, quantization, segmentation, loop filter, CDEF, loop
restoration and superres, but never bounds these reference indices.
Decoders use them directly as array subscripts. In the MediaTek decoder
vdec_av1_slice_setup_ref() does:
int ref_idx = ctrl_fh->ref_frame_idx[i];
pfc->ref_idx[i] = ctrl_fh->reference_frame_ts[ref_idx];
slot_id = frame->ref_frame_map[ref_idx];
indexing reference_frame_ts[8] and ref_frame_map[8] with an attacker
controlled signed value. In the Rockchip/verisilicon decoder the CDF setup
does:
rockchip_av1_get_cdfs(ctx,
frame->ref_frame_idx[frame->primary_ref_frame]);
indexing ref_frame_idx[7] with the unbounded primary_ref_frame. Both are
out-of-bounds reads driven by unvalidated userspace input.
Validate in the core, like the other AV1 frame fields, so every decoder is
covered: reject ref_frame_idx entries outside
[0, V4L2_AV1_TOTAL_REFS_PER_FRAME) and primary_ref_frame >=
V4L2_AV1_TOTAL_REFS_PER_FRAME. The upper bound still permits the value 7
(PRIMARY_REF_NONE), which decoders handle explicitly.
Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
Found by static analysis; not yet runtime-reproduced (Rockchip/MediaTek SoC
hardware required).
Fixes: 9de30f579980 ("media: Add AV1 uAPI")
Assisted-by: 0sec:claude-opus-4.8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v2: wrap a quoted decoder line in the commit message to satisfy checkpatch
(<=75 cols); no code change from v1.
v1: https://lore.kernel.org/linux-media/20260628134105.21001-1-doruk@0sec.ai/
drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index ba047d7d8601..5096c48ea402 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -793,6 +793,7 @@ static int validate_av1_film_grain(struct v4l2_ctrl_av1_film_grain *fg)
static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
{
int ret = 0;
+ u32 i;
ret = validate_av1_quantization(&f->quantization);
if (ret)
@@ -836,6 +837,14 @@ static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
if (f->superres_denom > GENMASK(2, 0) + 9)
return -EINVAL;
+ for (i = 0; i < ARRAY_SIZE(f->ref_frame_idx); i++)
+ if (f->ref_frame_idx[i] < 0 ||
+ f->ref_frame_idx[i] >= V4L2_AV1_TOTAL_REFS_PER_FRAME)
+ return -EINVAL;
+
+ if (f->primary_ref_frame >= V4L2_AV1_TOTAL_REFS_PER_FRAME)
+ return -EINVAL;
+
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] media: v4l2-ctrls: validate AV1 ref_frame_idx and primary_ref_frame
2026-07-09 13:12 [PATCH v2] media: v4l2-ctrls: validate AV1 ref_frame_idx and primary_ref_frame Doruk Tan Ozturk
@ 2026-09-28 20:51 ` Nicolas Dufresne
0 siblings, 0 replies; 2+ messages in thread
From: Nicolas Dufresne @ 2026-09-28 20:51 UTC (permalink / raw)
To: Doruk Tan Ozturk, hverkuil, mchehab
Cc: sakari.ailus, laurent.pinchart, detlev.casanova, kees,
michael.bommarito, linux-media, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3480 bytes --]
Le jeudi 09 juillet 2026 à 15:12 +0200, Doruk Tan Ozturk a écrit :
> The stateless AV1 frame control V4L2_CID_STATELESS_AV1_FRAME carries
> ref_frame_idx[V4L2_AV1_REFS_PER_FRAME] (signed, -128..127) and
> primary_ref_frame (__u8), both copied from userspace. validate_av1_frame()
> already checks flags, quantization, segmentation, loop filter, CDEF, loop
> restoration and superres, but never bounds these reference indices.
>
> Decoders use them directly as array subscripts. In the MediaTek decoder
> vdec_av1_slice_setup_ref() does:
>
> int ref_idx = ctrl_fh->ref_frame_idx[i];
> pfc->ref_idx[i] = ctrl_fh->reference_frame_ts[ref_idx];
> slot_id = frame->ref_frame_map[ref_idx];
>
> indexing reference_frame_ts[8] and ref_frame_map[8] with an attacker
> controlled signed value. In the Rockchip/verisilicon decoder the CDF setup
> does:
>
> rockchip_av1_get_cdfs(ctx,
> frame->ref_frame_idx[frame->primary_ref_frame]);
>
> indexing ref_frame_idx[7] with the unbounded primary_ref_frame. Both are
> out-of-bounds reads driven by unvalidated userspace input.
>
> Validate in the core, like the other AV1 frame fields, so every decoder is
> covered: reject ref_frame_idx entries outside
> [0, V4L2_AV1_TOTAL_REFS_PER_FRAME) and primary_ref_frame >=
> V4L2_AV1_TOTAL_REFS_PER_FRAME. The upper bound still permits the value 7
> (PRIMARY_REF_NONE), which decoders handle explicitly.
>
> Found by 0sec's autonomous vulnerability analysis (https://0sec.ai).
> Found by static analysis; not yet runtime-reproduced (Rockchip/MediaTek SoC
> hardware required).
>
> Fixes: 9de30f579980 ("media: Add AV1 uAPI")
> Assisted-by: 0sec:claude-opus-4.8
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
> v2: wrap a quoted decoder line in the commit message to satisfy checkpatch
> (<=75 cols); no code change from v1.
> v1: https://lore.kernel.org/linux-media/20260628134105.21001-1-doruk@0sec.ai/
>
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index ba047d7d8601..5096c48ea402 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -793,6 +793,7 @@ static int validate_av1_film_grain(struct v4l2_ctrl_av1_film_grain *fg)
> static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
> {
> int ret = 0;
> + u32 i;
>
> ret = validate_av1_quantization(&f->quantization);
> if (ret)
> @@ -836,6 +837,14 @@ static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
> if (f->superres_denom > GENMASK(2, 0) + 9)
> return -EINVAL;
>
> + for (i = 0; i < ARRAY_SIZE(f->ref_frame_idx); i++)
> + if (f->ref_frame_idx[i] < 0 ||
> + f->ref_frame_idx[i] >= V4L2_AV1_TOTAL_REFS_PER_FRAME)
> + return -EINVAL;
This is likely a breaking changes, and since you didn't test it I'm being very
cautious. Through code inspection, I see that GStreamer will fill the
ref_frame_idx to value -1 when reference is unused. This happens notably on
every keyframe.
> +
> + if (f->primary_ref_frame >= V4L2_AV1_TOTAL_REFS_PER_FRAME)
> + return -EINVAL;
As per spec, this shouldn't be bigger then AV1_PRIMARY_REF_NONE (7). But it does
seems that passing values above can cause issues in
rockchip_vpu981_av1_dec_set_prob() notably.
Nicolas
> +
> return 0;
> }
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-28 20:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-09 13:12 [PATCH v2] media: v4l2-ctrls: validate AV1 ref_frame_idx and primary_ref_frame Doruk Tan Ozturk
2026-09-28 20:51 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®