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

[Geert Uytterhoeven]

Hi Wolfram,

On Mon, 23 Mar 2026 at 10:29, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> > > > > +       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.

Sorry, you are right.
I guess I was thinking the flags specified in DT could cause an invalid
index doubling, but those can indeed not happen due to the
priv->info->mb_* flag checks.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds