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

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Arnd Bergmann <arnd@kernel.org>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Jose Abreu <joabreu@synopsys.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Voon Weifeng <weifeng.voon@intel.com>,
	Ong Boon Leong <boon.leong.ong@intel.com>,
	Arnd Bergmann <arnd@arndb.de>, kernel test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [RESEND] net: stmmac: fix gcc-10 -Wrestrict warning
Date: Mon, 27 Sep 2021 14:26:06 +0100	[thread overview]
Message-ID: <YVHGblt9rYg7kbWR@shell.armlinux.org.uk> (raw)
In-Reply-To: <20210927100336.1334028-1-arnd@kernel.org>

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!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel