Re: [PATCH v4] media: v4l2-isp: reject zero-sized parameter blocks

Hi David

On Thu, Aug 20, 2026 at 09:25:44PM +0100, David Carlier wrote:
> v4l2_isp_params_validate_buffer() walks the blocks of a parameters
> buffer by adding block->size to the current offset. A block whose
> type_info[] entry is an uninitialised hole passes every check on the
> way there: a size of 0 is not caught by the block->size > buffer_size
> test, and the match against the type info size compares 0 with the
> hole's own 0 and passes as well. The walk then makes no forward
> progress and loops forever.
>
> Drivers build their type_info[] arrays with designated initialisers
> indexed by their block type enumeration, so an enumerator left without
> an entry leaves a zeroed hole rather than failing the build. Drivers
> call the validator from vb2 .buf_prepare, so such a hole turns a
> VIDIOC_QBUF on the parameters video device into an unkillable task
> spinning with the queue mutex held.
>
> Reject a block whose type info entry is empty. The driver does not
> implement the type, so it cannot tell whether the block content is
> meaningful, and accepting it silently would leave that content
> unconstrained until a later kernel implements the type and starts
> validating it. The size match then always runs against a non-zero
> size, and no block can advance the walk by zero.
>
> Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
> Cc: [email protected]
> Suggested-by: Jacopo Mondi <[email protected]>
> Signed-off-by: David Carlier <[email protected]>

Thank you for the new version

Reviewed-by: Jacopo Mondi <[email protected]>

> ---
> v4:
> - drop the block->size < sizeof(*block) check, redundant now that an
>   empty type info entry is rejected before the size match (Jacopo)
> - commit message reworked around the type info check
>
> v3:
> - reject a block whose type info entry is empty instead of skipping
>   it, so a type the driver does not implement cannot become
>   unconstrained uAPI (Jacopo)
>
> v2:
> - skip an empty type info entry instead of matching the block against
>   a zeroed one
> - reworded the commit message, which no longer leans on rppx1
>
>  drivers/media/v4l2-core/v4l2-isp.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
> index 1eb46e080afa..8e6c2ef326aa 100644
> --- a/drivers/media/v4l2-core/v4l2-isp.c
> +++ b/drivers/media/v4l2-core/v4l2-isp.c
> @@ -99,12 +99,25 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
>  			return -EINVAL;
>  		}
>
> +		/*
> +		 * An empty type info entry denotes a block type the driver
> +		 * does not support. Reject the buffer instead of ignoring the
> +		 * block: accepting it silently would let userspace fill it
> +		 * with data that a later kernel, once it implements the type,
> +		 * would validate and possibly reject.
> +		 */
> +		info = &type_info[block->type];
> +		if (!info->size) {
> +			dev_dbg(dev, "Unsupported block type %u at offset %zu\n",
> +				block->type, block_offset);
> +			return -EINVAL;
> +		}
> +
>  		/*
>  		 * Match the block reported size against the type info provided
>  		 * one, but allow the block to only contain the header in
>  		 * case it is going to be disabled.
>  		 */
> -		info = &type_info[block->type];
>  		if (block->size != info->size &&
>  		    (!(block->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) ||
>  		    block->size != sizeof(*block))) {
> --
> 2.55.0
>