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

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

[David Carlier]

v4l2_isp_params_validate_buffer() advances by block->size but never
bounds it from below. A block with size 0 passes both the
block->size > buffer_size test and the comparison against info->size
whenever the driver's type_info[] entry is an uninitialised hole, so
the walk makes no forward progress and loops forever.

Drivers call this from vb2 .buf_prepare, so VIDIOC_QBUF on the
parameters video device hangs the calling task with the queue mutex
held. rppx1 has such a hole today; fix the core so the walk terminates
for any driver.

Require every block to be at least as large as its header. Blocks
carrying only a header to disable a block are exactly that size and
still pass.

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

diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
index 1eb46e080afa..760e16a5ba9d 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;
-- 
2.55.0

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

[Jacopo Mondi]

Hi David

On Sat, Aug 15, 2026 at 08:38:39PM +0100, David Carlier wrote:
> v4l2_isp_params_validate_buffer() advances by block->size but never
> bounds it from below. A block with size 0 passes both the
> block->size > buffer_size test and the comparison against info->size
> whenever the driver's type_info[] entry is an uninitialised hole, so

This shouldn't happen (an empty type_info[] I mean)

> the walk makes no forward progress and loops forever.
>
> Drivers call this from vb2 .buf_prepare, so VIDIOC_QBUF on the
> parameters video device hangs the calling task with the queue mutex
> held. rppx1 has such a hole today; fix the core so the walk terminates

Uh, where ?

I see

static const struct v4l2_isp_params_block_type_info
rppx1_ext_params_blocks_info[] = {
	RPPX1_PARAMS_BLOCK_INFO(BLS_PRE1, bls),
	RPPX1_PARAMS_BLOCK_INFO(BLS_PRE2, bls),
	RPPX1_PARAMS_BLOCK_INFO(LIN_PRE1, lin),
	RPPX1_PARAMS_BLOCK_INFO(LIN_PRE2, lin),
	RPPX1_PARAMS_BLOCK_INFO(LSC_PRE1, lsc),
	RPPX1_PARAMS_BLOCK_INFO(LSC_PRE2, lsc),
	RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE1, awbg),
	RPPX1_PARAMS_BLOCK_INFO(AWBG_PRE2, awbg),
	RPPX1_PARAMS_BLOCK_INFO(CCOR_POST, ccor),
	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE1, hist),
	RPPX1_PARAMS_BLOCK_INFO(HIST_PRE2, hist),
	RPPX1_PARAMS_BLOCK_INFO(HIST_POST, hist),
	RPPX1_PARAMS_BLOCK_INFO(EXM_PRE1, exm),
	RPPX1_PARAMS_BLOCK_INFO(EXM_PRE2, exm),
	RPPX1_PARAMS_BLOCK_INFO(WBMEAS_POST, wbmeas),
	RPPX1_PARAMS_BLOCK_INFO(GA_HV, ga),
	RPPX1_PARAMS_BLOCK_INFO(GA_MV, ga),
};

being passed to v4l2_isp_params_validate_buffer().

> for any driver.
>
> Require every block to be at least as large as its header. Blocks
> carrying only a header to disable a block are exactly that size and
> still pass.
>
> Fixes: 3cb6de6fafb8 ("media: v4l2-core: Introduce v4l2-isp.c")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>  drivers/media/v4l2-core/v4l2-isp.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-isp.c b/drivers/media/v4l2-core/v4l2-isp.c
> index 1eb46e080afa..760e16a5ba9d 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 not against this, but it only makes sense if the driver populates
the list of v4l2_isp_params_block_type_info[] with an empty item,
which shouldn't happen.

>  		if (block->size > buffer_size) {
>  			dev_dbg(dev, "Premature end of parameters data\n");
>  			return -EINVAL;
> --
> 2.55.0
>

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

[David CARLIER]

Hi Jacopo,

On Mon, Aug 17, 2026 at 11:59:15AM +0200, Jacopo Mondi wrote:
> > whenever the driver's type_info[] entry is an uninitialised hole, so
>
> This shouldn't happen (an empty type_info[] I mean)
[...]
> > held. rppx1 has such a hole today; fix the core so the walk terminates
>
> Uh, where ?

Sorry, I should have spelled this out in the commit message.

RPPX1_PARAMS_BLOCK_INFO() is a designated initialiser indexed by the
block type (rpp_params.c:13):

      #define RPPX1_PARAMS_BLOCK_INFO(block, data) \
              [RPPX1_PARAMS_BLOCK_TYPE_ ## block] = { \
                      .size = sizeof(struct rppx1_ ## data ## _params), \
              }

so the list reads as dense but is indexed by the enum. It has 17
entries for 18 enumerators, and the missing one is AWBG_POST (== 3).
The last index used is LIN_PRE2 (== 17), so ARRAY_SIZE() is still 18
and the entry at 3 is simply zeroed.

A block with type AWBG_POST and size 0 then gets through: 0 is not
larger than the remaining buffer, and block->size != info->size is
0 != 0. block_offset and buffer_size don't move and the loop spins,
in .buf_prepare, holding the queue mutex.

> I'm not against this, but it only makes sense if the driver populates
> the list of v4l2_isp_params_block_type_info[] with an empty item,
> which shouldn't happen.

True, and rppx1 is the only one that does - rkisp1, c3-isp and
mali-c55 all look complete. What bothers me is that the loop only
terminates if every driver's table is right, and nothing checks that
at build time. A missing line in a driver would be a rejected buffer
rather than a stuck task. And a block size below its own header is
malformed anyway, whatever the type.

That said, rppx1 needs a patch either way: AWBG_POST is documented i
rppx1-config.h and the driver probes rpp->post.awbg, but the block
can't be used today - the size check compares against 0, and
rppx1_params() has no case for it in the switch.

Happy to drop a separate patch and keep only the driver fix if you prefer.

Cheers.