| Message ID | 20210927100336.1334028-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND] net: stmmac: fix gcc-10 -Wrestrict warning | expand |
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Mon, 27 Sep 2021 12:02:44 +0200 you wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-10 and later warn about a theoretical array overrun when > accessing priv->int_name_rx_irq[i] with an out of bounds value > of 'i': > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function 'stmmac_request_irq_multi_msi': > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3528:17: error: 'snprintf' argument 4 may overlap destination object 'dev' [-Werror=restrict] > 3528 | snprintf(int_name, int_name_len, "%s:%s-%d", dev->name, "tx", i); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3404:60: note: destination object referenced by 'restrict'-qualified argument 1 was declared here > 3404 | static int stmmac_request_irq_multi_msi(struct net_device *dev) > | ~~~~~~~~~~~~~~~~~~~^~~ > > [...] Here is the summary with links: - [RESEND] net: stmmac: fix gcc-10 -Wrestrict warning https://git.kernel.org/netdev/net-next/c/3e0d5699a975 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Mon, Sep 27, 2021 at 12:02:44PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gcc-10 and later warn about a theoretical array overrun when > accessing priv->int_name_rx_irq[i] with an out of bounds value > of 'i': > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c: In function 'stmmac_request_irq_multi_msi': > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3528:17: error: 'snprintf' argument 4 may overlap destination object 'dev' [-Werror=restrict] > 3528 | snprintf(int_name, int_name_len, "%s:%s-%d", dev->name, "tx", i); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:3404:60: note: destination object referenced by 'restrict'-qualified argument 1 was declared here > 3404 | static int stmmac_request_irq_multi_msi(struct net_device *dev) > | ~~~~~~~~~~~~~~~~~~~^~~ > > The warning is a bit strange since it's not actually about the array > bounds but rather about possible string operations with overlapping > arguments, but it's not technically wrong. > > Avoid the warning by adding an extra bounds check. > > Fixes: 8532f613bc78 ("net: stmmac: introduce MSI Interrupt routines for mac, safety, RX & TX") > Link: https://lore.kernel.org/all/20210421134743.3260921-1-arnd@kernel.org/ > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index 553c4403258a..640c0ffdff3d 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -3502,6 +3502,8 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) > > /* Request Rx MSI irq */ > for (i = 0; i < priv->plat->rx_queues_to_use; i++) { > + if (i > MTL_MAX_RX_QUEUES) > + break; > if (priv->rx_irq[i] == 0) > continue; This looks rather weird. rx_irq[] is defined as: int rx_irq[MTL_MAX_RX_QUEUES]; If "i" were to become MTL_MAX_RX_QUEUES, then the above code overlows the array. So while this may stop gcc-10 complaining, I'd argue that making the new test ">=" rather than ">" would have also made it look correct. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Sep 27, 2021 at 3:27 PM Russell King (Oracle) <linux@armlinux.org.uk> wrote: > On Mon, Sep 27, 2021 at 12:02:44PM +0200, Arnd Bergmann wrote: > > This looks rather weird. rx_irq[] is defined as: > > int rx_irq[MTL_MAX_RX_QUEUES]; > > If "i" were to become MTL_MAX_RX_QUEUES, then the above code overlows > the array. > > So while this may stop gcc-10 complaining, I'd argue that making the > new test ">=" rather than ">" would have also made it look correct. Indeed, thanks for pointing this out. I have sent a follow-up with that change now. Arnd
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index 553c4403258a..640c0ffdff3d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3502,6 +3502,8 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) /* Request Rx MSI irq */ for (i = 0; i < priv->plat->rx_queues_to_use; i++) { + if (i > MTL_MAX_RX_QUEUES) + break; if (priv->rx_irq[i] == 0) continue; @@ -3525,6 +3527,8 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev) /* Request Tx MSI irq */ for (i = 0; i < priv->plat->tx_queues_to_use; i++) { + if (i > MTL_MAX_TX_QUEUES) + break; if (priv->tx_irq[i] == 0) continue;