On Thu, Jun 17, 2021 at 02:14:06PM +0000, Prabhakar Mahadev Lad wrote:
> > 223 dev_err(dev, "Invalid %s clock index %u\n", type, clkidx);
> > 224 return ERR_PTR(-EINVAL);
> > 225 }
> > 226 clk = priv->clks[clkidx];
> > 227 break;
> > 228
> > 229 case CPG_MOD:
> > 230 type = "module";
> > 231 if (clkidx > priv->num_mod_clks) {
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > Smatch did not catch it, but this condition is definitely off by one. ;)
> >
> Good catch this definitely needs to be if (clkidx > (priv->num_mod_clks - 1)
The size - 1 format is riskier because there is a potential for
underflow.
Imagine that in the future priv->num_mod_clks is zero.
"priv->num_mod_clks - 1" is now UINT_MAX and any value of "clkidx" is
accepted. In this case, you know that the value of num_mod_clks if 57
but it took me some time to figure that out and ensure that it couldn't
be zero.
regards,
dan carpenter
