Re: [PATCH] media: coda: fix out-of-bounds read when parsing H.264 SPS profile

[Philipp Zabel]

On Do, 2026-11-05 at 15:41 +0900, Hyunjung Ko wrote:
> coda_sps_parse_profile() scans the compressed OUTPUT-plane buffer
> supplied by userspace via VIDIOC_QBUF for an SPS NAL unit and then reads
> the profile_idc (buf[0]) and level_idc (buf[2]) that follow the NAL unit
> header byte. Neither the NAL scanning helper nor the final dereferences
> are bounded against the end of the payload:
> 
>   - coda_find_nal_header() dereferences *buf++ before checking buf
>     against end, so it reads one byte past the payload when it is
>     entered with buf == end (e.g. a zero-length or start-code-terminated
>     buffer).
> 
>   - after the SPS NAL unit header byte is consumed, buf may point at or
>     near the end of the payload, yet buf[0] and buf[2] are read with no
>     check that three bytes are actually available, reading up to three
>     bytes past the end of the payload (and past the allocation when
>     bytesused == plane size).
> 
> A local user with access to /dev/videoN can trigger this by queueing a
> crafted H.264 bitstream buffer ending right after an SPS start code and
> NAL header byte, resulting in an out-of-bounds read (KASAN splat /
> possible crash).
> 
> Rewrite coda_find_nal_header() to bound-check before each read, reject a
> header found with no following byte, and verify that profile_idc and
> level_idc are inside the payload before dereferencing them.
> 
> Signed-off-by: Hyunjung Ko <hj351016@xxxxxxxxx>
> ---
>  drivers/media/platform/chips-media/coda/coda-h264.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/chips-media/coda/coda-h264.c b/drivers/media/platform/chips-media/coda/coda-h264.c
> --- a/drivers/media/platform/chips-media/coda/coda-h264.c
> +++ b/drivers/media/platform/chips-media/coda/coda-h264.c
> @@ -19,13 +19,13 @@
>  {
>  	u32 val = 0xffffffff;
> 
> -	do {
> +	while (buf < end) {
>  		val = val << 8 | *buf++;
> -		if (buf >= end)
> -			return NULL;
> -	} while (val != 0x00000001);
> +		if (val == 0x00000001)
> +			return buf;

This should return NULL if buf >= end ...

> +	}
> 
> -	return buf;
> +	return NULL;
>  }
> 
>  int coda_sps_parse_profile(struct coda_ctx *ctx, struct vb2_buffer *vb)
> @@ -36,10 +36,17 @@
>  	/* Find SPS header */
>  	do {
>  		buf = coda_find_nal_header(buf, end);
> -		if (!buf)
> +		if (!buf || buf >= end)

... and this should not be needed.

coda_find_nal_header() should never return buf >= end.

>  			return -EINVAL;
>  	} while ((*buf++ & 0x1f) != 0x7);
> 
> +	/*
> +	 * profile_idc (buf[0]) and level_idc (buf[2]) must lie within the
> +	 * payload before they can be dereferenced.
> +	 */
> +	if (end - buf < 3)
> +		return -EINVAL;
> +
>  	ctx->params.h264_profile_idc = buf[0];
>  	ctx->params.h264_level_idc = buf[2];
> 
> --
> 2.43.0

With the above fixed,

Reviewed-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>

regards
Philipp