netdev - Re: [PATCH v2 net-next 10/13] net: enetc: extract enetc_int_vector_init/destroy() from enetc_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241016072929.GD2162@kernel.org>
Date: Wed, 16 Oct 2024 08:29:29 +0100
From: Simon Horman <horms@...nel.org>
To: Wei Fang <wei.fang@....com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, vladimir.oltean@....com,
	claudiu.manoil@....com, xiaoning.wang@....com, Frank.Li@....com,
	christophe.leroy@...roup.eu, linux@...linux.org.uk,
	bhelgaas@...gle.com, imx@...ts.linux.dev, netdev@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH v2 net-next 10/13] net: enetc: extract
 enetc_int_vector_init/destroy() from enetc_alloc_msix()

On Tue, Oct 15, 2024 at 08:58:38PM +0800, Wei Fang wrote:
> From: Clark Wang <xiaoning.wang@....com>
> 
> Extract enetc_int_vector_init() and enetc_int_vector_destroy() from
> enetc_alloc_msix() so that the code is more concise and readable.
> 
> Signed-off-by: Clark Wang <xiaoning.wang@....com>
> Signed-off-by: Wei Fang <wei.fang@....com>
> ---
> v2 changes:
> This patch is separated from v1 patch 9 ("net: enetc: optimize the
> allocation of tx_bdr"). Separate enetc_int_vector_init() from the
> original patch. In addition, add new help function
> enetc_int_vector_destroy().
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 174 +++++++++----------
>  1 file changed, 87 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 032d8eadd003..d36af3f8ba31 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -2965,6 +2965,87 @@ int enetc_ioctl(struct net_device *ndev, struct ifreq *rq, int cmd)
>  }
>  EXPORT_SYMBOL_GPL(enetc_ioctl);
>  
> +static int enetc_int_vector_init(struct enetc_ndev_priv *priv, int i,
> +				 int v_tx_rings)
> +{
> +	struct enetc_int_vector *v __free(kfree);
> +	struct enetc_bdr *bdr;
> +	int j, err;
> +
> +	v = kzalloc(struct_size(v, tx_ring, v_tx_rings), GFP_KERNEL);
> +	if (!v)
> +		return -ENOMEM;

...

>  int enetc_alloc_msix(struct enetc_ndev_priv *priv)
>  {
>  	struct pci_dev *pdev = priv->si->pdev;

...

> @@ -2986,64 +3067,9 @@ int enetc_alloc_msix(struct enetc_ndev_priv *priv)
>  	/* # of tx rings per int vector */
>  	v_tx_rings = priv->num_tx_rings / priv->bdr_int_num;
>  
> -	for (i = 0; i < priv->bdr_int_num; i++) {
> -		struct enetc_int_vector *v;
> -		struct enetc_bdr *bdr;
> -		int j;
> -
> -		v = kzalloc(struct_size(v, tx_ring, v_tx_rings), GFP_KERNEL);
> -		if (!v) {
> -			err = -ENOMEM;
> +	for (i = 0; i < priv->bdr_int_num; i++)
> +		if (enetc_int_vector_init(priv, i, v_tx_rings))
>  			goto fail;

Hi Wei Fang,

It looks like, if we reach this error handling during the first iteration
of the for loop then err, which will be return value returned by the function,
is ininitialised. Perhaps this would be better expressed as follows?
(Completely untested!)

		err = enetc_int_vector_init(priv, i, v_tx_rings);
		if (err)
			goto fail;

Flagged by Smatch.

...