Re: [PATCH] [RESEND] net: stmmac: fix gcc-10 -Wrestrict warning

[Russell King (Oracle)]

On Mon, Sep 27, 2021 at 12:02:44PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
> 
> 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@xxxxxxxxxx/
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>  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!