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

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

[David Carlier]

v4l2_isp_params_validate_buffer() walks the blocks of a parameters
buffer by adding block->size to the current offset, but never bounds
that size from below. A block with size 0 is not caught by the
block->size > buffer_size test, and the comparison against info->size
passes as well when the driver's type_info[] entry is an uninitialised
hole, both sizes being 0. 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 smaller than its own header. A block's size includes its
header, so anything below that is malformed whatever the driver table
contains, and rejecting it is what keeps the walk moving. Blocks
carrying only a header to disable a block are exactly that size and
still pass.

An empty type info entry can then no longer stall the walk, but a block
matched against it is only constrained by that header size check. Reject
such a block explicitly: the driver does not implement the type and
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.

Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
Cc: stable@vger.kernel.org
Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: David Carlier <devnexen@gmail.com>
---
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 | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 1eb46e080afa..efe994b4c4d7 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -84,6 +84,13 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 			return -EINVAL;
 		}
 
+		if (block->size < sizeof(*block)) {
+			dev_dbg(dev,
+				"Invalid block size %u at offset %zu\n",
+				block->size, block_offset);
+			return -EINVAL;
+		}
+
 		if (block->size > buffer_size) {
 			dev_dbg(dev, "Premature end of parameters data\n");
 			return -EINVAL;
@@ -99,12 +106,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

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

[Jacopo Mondi]

Hi David

On Thu, Aug 20, 2026 at 12:17:22PM +0100, David Carlier wrote:
> v4l2_isp_params_validate_buffer() walks the blocks of a parameters
> buffer by adding block->size to the current offset, but never bounds
> that size from below. A block with size 0 is not caught by the
> block->size > buffer_size test, and the comparison against info->size
> passes as well when the driver's type_info[] entry is an uninitialised
> hole, both sizes being 0. 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 smaller than its own header. A block's size includes its
> header, so anything below that is malformed whatever the driver table
> contains, and rejecting it is what keeps the walk moving. Blocks
> carrying only a header to disable a block are exactly that size and
> still pass.
>
> An empty type info entry can then no longer stall the walk, but a block
> matched against it is only constrained by that header size check. Reject
> such a block explicitly: the driver does not implement the type and
> 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.
>
> Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
> Cc: stable@vger.kernel.org
> Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
> 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 | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
> index 1eb46e080afa..efe994b4c4d7 100644
> --- a/drivers/media/v4l2-core/v4l2-isp.c
> +++ b/drivers/media/v4l2-core/v4l2-isp.c
> @@ -84,6 +84,13 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
>  			return -EINVAL;
>  		}
>
> +		if (block->size < sizeof(*block)) {
> +			dev_dbg(dev,
> +				"Invalid block size %u at offset %zu\n",
> +				block->size, block_offset);
> +			return -EINVAL;
> +		}
> +

I'm sorry, but now that we refuse empty block info with size == 0,
wouldn't this be caught by the below

		if (block->size != info->size &&
		    (!(block->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) ||
		    block->size != sizeof(*block))) {

Do we need to check it here as well ?

nit: the dev_dbg() line fits on 2 lines only.


>  		if (block->size > buffer_size) {
>  			dev_dbg(dev, "Premature end of parameters data\n");
>  			return -EINVAL;
> @@ -99,12 +106,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
>

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

[David Carlier]

v4l2_isp_params_validate_buffer() walks the blocks of a parameters
buffer by adding block->size to the current offset, but never bounds
that size from below. A block with size 0 is not caught by the
block->size > buffer_size test, and the comparison against info->size
passes as well when the driver's type_info[] entry is an uninitialised
hole, both sizes being 0. 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 smaller than its own header. A block's size includes its
header, so anything below that is malformed whatever the driver table
contains, and rejecting it is what keeps the walk moving. Blocks
carrying only a header to disable a block are exactly that size and
still pass.

An empty type info entry can then no longer stall the walk, so skip
such a block instead of failing the whole buffer: drivers may reserve
uAPI block types they do not implement yet, and are free to ignore the
block when processing the buffer.

Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
Cc: stable@vger.kernel.org
Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: David Carlier <devnexen@gmail.com>
---
v2:
- skip a block whose type info entry is empty instead of
  matching it against a zeroed entry, so a type the driver
  does not implement is ignored rather than failing the
  whole buffer (Jacopo)
- reworded the commit message, which no longer leans on
  rppx1: its missing AWBG_POST entry is being fixed at
  https://patchwork.linuxtv.org/project/linux-media/list/?series=29170

 drivers/media/v4l2-core/v4l2-isp.c | 41 ++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 1eb46e080afa..439477b2b941 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -84,6 +84,13 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 			return -EINVAL;
 		}
 
+		if (block->size < sizeof(*block)) {
+			dev_dbg(dev,
+				"Invalid block size %u at offset %zu\n",
+				block->size, block_offset);
+			return -EINVAL;
+		}
+
 		if (block->size > buffer_size) {
 			dev_dbg(dev, "Premature end of parameters data\n");
 			return -EINVAL;
@@ -100,23 +107,31 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 		}
 
 		/*
-		 * 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.
+		 * An empty type info entry denotes a block type the driver
+		 * does not support. Skip the block, it is up to the driver to
+		 * ignore it when processing the buffer.
 		 */
 		info = &type_info[block->type];
-		if (block->size != info->size &&
-		    (!(block->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) ||
-		    block->size != sizeof(*block))) {
-			dev_dbg(dev,
-				"Invalid block size %u (expected %zu) at offset %zu\n",
-				block->size, info->size, block_offset);
-			return -EINVAL;
+		if (info->size) {
+			/*
+			 * 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.
+			 */
+			if (block->size != info->size &&
+			    (!(block->flags & V4L2_ISP_PARAMS_FL_BLOCK_DISABLE) ||
+			    block->size != sizeof(*block))) {
+				dev_dbg(dev,
+					"Invalid block size %u (expected %zu) at offset %zu\n",
+					block->size, info->size, block_offset);
+				return -EINVAL;
+			}
+
+			if (info->block_validate &&
+			    info->block_validate(dev, block))
+				return -EINVAL;
 		}
 
-		if (info->block_validate && info->block_validate(dev, block))
-			return -EINVAL;
-
 		block_offset += block->size;
 		buffer_size -= block->size;
 	}
-- 
2.55.0

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

[David Carlier]

v4l2_isp_params_validate_buffer() walks the blocks of a parameters
buffer by adding block->size to the current offset, but never bounds
that size from below. A block with size 0 is not caught by the
block->size > buffer_size test, and the comparison against info->size
passes as well when the driver's type_info[] entry is an uninitialised
hole, both sizes being 0. 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 smaller than its own header. A block's size includes its
header, so anything below that is malformed whatever the driver table
contains, and rejecting it is what keeps the walk moving. Blocks
carrying only a header to disable a block are exactly that size and
still pass.

An empty type info entry can then no longer stall the walk, but a block
matched against it is only constrained by that header size check. Reject
such a block explicitly: the driver does not implement the type and
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.

Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
Cc: stable@vger.kernel.org
Suggested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/media/v4l2-core/v4l2-isp.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 1eb46e080afa..efe994b4c4d7 100644
--- a/drivers/media/v4l2-core/v4l2-isp.c
+++ b/drivers/media/v4l2-core/v4l2-isp.c
@@ -84,6 +84,13 @@ int v4l2_isp_params_validate_buffer(struct device *dev, struct vb2_buffer *vb,
 			return -EINVAL;
 		}
 
+		if (block->size < sizeof(*block)) {
+			dev_dbg(dev,
+				"Invalid block size %u at offset %zu\n",
+				block->size, block_offset);
+			return -EINVAL;
+		}
+
 		if (block->size > buffer_size) {
 			dev_dbg(dev, "Premature end of parameters data\n");
 			return -EINVAL;
@@ -99,12 +106,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

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

[David CARLIER]

Oupsie, sent with the wrong subject prefix and without its
changelog. It has:

v3:
- reject a block whose type info entry is empty instead of skipping it,
  so an unimplemented type does not become unconstrained uAPI

Let me know if you want it resent properly labelled.

Cheers.

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

[Jacopo Mondi]

Hi David

On Wed, Aug 19, 2026 at 10:41:27PM +0100, David CARLIER wrote:
> Oupsie, sent with the wrong subject prefix and without its

The subject seems right to me ?

More than this, you sent v3 in reply to v2 which I think possibly
confuses tooling (not patchwork though, which correctly identifies
this one as a separate series [1])

Would you like to resend as a separate thread ? Sorry for bothering

[1] https://patchwork.linuxtv.org/project/linux-media/patch/20260819213459.1220894-1-devnexen@gmail.com/

> changelog. It has:
>
> v3:
> - reject a block whose type info entry is empty instead of skipping it,
>   so an unimplemented type does not become unconstrained uAPI
>
> Let me know if you want it resent properly labelled.
>
> Cheers.
>