Hi Jacopo,
On Mon, Jul 08, 2024 at 10:25:34AM +0200, Jacopo Mondi wrote:
> > > +static void rkisp1_ext_params_config(struct rkisp1_params *params,
> > > + struct rkisp1_ext_params_cfg *cfg,
> > > + u32 block_group_mask)
> > > +{
> > > + size_t block_offset = 0;
> > > +
> > > + if (WARN_ON(!cfg))
> > > + return;
> > > +
> > > + /* Walk the list of parameter blocks and process them. */
> > > + while (block_offset < cfg->data_size) {
> > > + const struct rkisp1_ext_params_handler *block_handler;
> > > + const union rkisp1_ext_params_config *block;
> > > +
> > > + block = (const union rkisp1_ext_params_config *)
> > > + &cfg->data[block_offset];
> >
> > In validation, you only check that if full headers exist, then headers are
> > fine. But here you don't perform that check, meaning you may have partial
> > headers here only. Either check here, too, or check that there's no more
> > data after the last block during validation.
>
> My preference would be for checking during validation that there is
> no data after the last valid header.
>
> I think:
>
> @@ -2438,6 +2438,12 @@ static int rkisp1_params_prepare_ext_params(struct rkisp1_params *params,
> cfg_size -= block->size;
> }
>
> + if (cfg_size) {
> + dev_dbg(params->rkisp1->dev,
> + "Unexpected data after the parameters buffer end\n");
> + return -EINVAL;
> + }
> +
> return 0;
> }
>
> would do ?
I believe it would. I also think it's better to address this during the
validation.
--
Kind regards,
Sakari Ailus
