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

Hi David

On Mon, Aug 17, 2026 at 02:40:53PM +0100, David CARLIER wrote:
> 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

Upsie, you're right, I didn't properly consider that..


> 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

Right.

Should we instead deman that all entries provided by the driver are
populated by failing validation if info->size == 0 ?

> 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.

Right.. you might have seen ?
https://patchwork.linuxtv.org/project/linux-media/list/?series=29170

In any case, yes, I think we should populate all entries and either
require all info blocks to be valid (hence a 0 sized block from
userspace is refused) or explicitly check if block->size > 0
as you're doing here.


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