[kbuild] Re: drivers/net/dsa/qca8k.c:944 qca8k_parse_port_config() error: testing array offset 'cpu_port_index' after use.

2021-11-16 Thread Dan Carpenter
On Tue, Nov 16, 2021 at 02:30:59PM +0100, Ansuel Smith wrote:
> > On Mon, Nov 15, 2021 at 07:08:30PM +0100, Ansuel Smith wrote:
> > > > tree:   
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git   
> > > > master
> > > > head:   debe436e77c72fcee804fb867f275e6d31aa999c
> > > > commit: 5654ec78dd7e64b1e04777b24007344329e6a63b net: dsa: qca8k: 
> > > > rework rgmii delay logic and scan for cpu port 6
> > > > config: i386-randconfig-m021-20211025 (attached as .config)
> > > > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot 
> > > > Reported-by: Dan Carpenter 
> > > >
> > >
> > > This should already be fixed by 06dd34a628ae5b6a839b757e746de165d6789ca8
> > > Can you confirm this?
> > >
> >
> > No, it doesn't fix the problem.  The check is either useless and should
> > be removed or there is an out of bounds bug.  Checking for an out of
> > bounds *after* you've already written to the memory is *never* useful.
> >
> > regards,
> > dan carpenter
> >
> 
> Again sorry if I insist and I'm 99% missing something.
> There was an out of bounds bug.
> It was fixed by 06dd34a628ae5b6a839b757e746de165d6789ca8 by
> starting the counter to -1.
> The extra check is useless, yes.

Then just delete it.  We don't want code which is illogical even if it
has no effect on run time.

regards,
dan carpenter
___
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-le...@lists.01.org


[kbuild] Re: drivers/net/dsa/qca8k.c:944 qca8k_parse_port_config() error: testing array offset 'cpu_port_index' after use.

2021-11-15 Thread Dan Carpenter
On Mon, Nov 15, 2021 at 07:08:30PM +0100, Ansuel Smith wrote:
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git  
> > master
> > head:   debe436e77c72fcee804fb867f275e6d31aa999c
> > commit: 5654ec78dd7e64b1e04777b24007344329e6a63b net: dsa: qca8k: rework 
> > rgmii delay logic and scan for cpu port 6
> > config: i386-randconfig-m021-20211025 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot 
> > Reported-by: Dan Carpenter 
> >
> 
> This should already be fixed by 06dd34a628ae5b6a839b757e746de165d6789ca8
> Can you confirm this?
> 

No, it doesn't fix the problem.  The check is either useless and should
be removed or there is an out of bounds bug.  Checking for an out of
bounds *after* you've already written to the memory is *never* useful.

regards,
dan carpenter

> > smatch warnings:
> > drivers/net/dsa/qca8k.c:944 qca8k_parse_port_config() error: testing array 
> > offset 'cpu_port_index' after use.
> >
> > vim +/cpu_port_index +944 drivers/net/dsa/qca8k.c
> >
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   934  static int
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   935  
> > qca8k_parse_port_config(struct qca8k_priv *priv)
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   936  {
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   937int port, cpu_port_index = 
> > 0, ret;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   938struct device_node *port_dn;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   939phy_interface_t mode;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   940struct dsa_port *dp;
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   941u32 delay;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   942
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   943/* We have 2 CPU port. 
> > Check them */
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14  @944for (port = 0; port < 
> > QCA8K_NUM_PORTS && cpu_port_index < QCA8K_NUM_CPU_PORTS; port++) {
> > 
> >  ^
> > Assume cpu_port_index = QCA8K_NUM_CPU_PORTS - 1;
> >
> >
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   945/* Skip every other 
> > port */
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   946if (port != 0 && 
> > port != 6)
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   947continue;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   948
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   949dp = 
> > dsa_to_port(priv->ds, port);
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   950port_dn = dp->dn;
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   951cpu_port_index++;
> > ^
> > cpu_port_index is now out of bounds.
> >
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   952
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   953if 
> > (!of_device_is_available(port_dn))
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   954continue;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   955
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   956ret = 
> > of_get_phy_mode(port_dn, );
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   957if (ret)
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   958continue;
> > 6c43809bf1bee7 Ansuel Smith 2021-10-14   959
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   960switch (mode) {
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   961case 
> > PHY_INTERFACE_MODE_RGMII:
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   962case 
> > PHY_INTERFACE_MODE_RGMII_ID:
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   963case 
> > PHY_INTERFACE_MODE_RGMII_TXID:
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   964case 
> > PHY_INTERFACE_MODE_RGMII_RXID:
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   965delay = 0;
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   966
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   967if 
> > (!of_property_read_u32(port_dn, "tx-internal-delay-ps", ))
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   968/* 
> > Switch regs accept value in ns, convert ps to ns */
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   969
> > delay = delay / 1000;
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   970else if 
> > (mode == PHY_INTERFACE_MODE_RGMII_ID ||
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   971 
> > mode == PHY_INTERFACE_MODE_RGMII_TXID)
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   972
> > delay = 1;
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   973
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   974if (delay > 
> > QCA8K_MAX_DELAY) {
> > 5654ec78dd7e64 Ansuel Smith 2021-10-14   975