Re: [PATCH 2/3] soc: renesas: Add Renesas R-Car MFIS driver

[Wolfram Sang]

> > > > +       if (priv->info->mb_reg_comes_from_dt) {
> > > > +               tx_uses_eicr = chan_flags & MFIS_CHANNEL_EICR;
> > > > +               if (tx_uses_eicr)
> > > > +                       chan += mbox->num_chans / 2;
> > > > +       } else {
> > > > +               tx_uses_eicr = priv->info->mb_tx_uses_eicr;
> > > > +       }
> > >
> > > "chan - mbox->chans" is the logical channel number, and should be
> > > validated against mbox_num_chans, to avoid out-of-bound accesses.
> >
> > "chan - ..."? You mean "chan + ..."?
> 
> No, I did mean "-": you do have a pointer "chan" to the channel,
> instead of an index into the mbox->chans[] array.
> Using a  index would  make validation easier to read, though.

Sorry, I still don't get it: If 'chan_num' has been sanitized above to
be in the range of 0..priv->info->mb_num_channels - 1, then how can a
OOB happen here? "chan += mbox->num_chans / 2" only happens when
'mb_reg_comes_from_dt' is set. But when it is set, the array of chans is
also always doubled to have the "<n> IICR, then <n> EICR" layout.

Attachment: signature.asc
Description: PGP signature