From: Arnd Bergmann <[email protected]>
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/[email protected]/
Reported-by: kernel test robot <[email protected]>
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
---
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;
@@ -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;
--
2.29.2
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 <[email protected]>
>
> 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 <[email protected]>
>
> 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/[email protected]/
> Reported-by: kernel test robot <[email protected]>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> 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)
<[email protected]> 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