mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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®