Skip to contentSkip to conversation

crypto: caam: fix netdev memory leak in dpaa2_caam_probe

PATCH In review Jianpeng Chang 3 participants 4 messages Jan 20 – Jan 31

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 crypto: caam: fix netdev memory leak in dpaa2_caam_probe

When commit 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2") converted embedded net_device to dynamically allocated pointers, it added cleanup in dpaa2_dpseci_disable() but missed adding cleanup in dpaa2_dpseci_free() for error paths.

This causes memory leaks when dpaa2_dpseci_dpio_setup() fails during probe due to DPIO devices not being ready yet. The kernel's deferred probe mechanism handles the retry successfully, but the netdevs allocated during the failed probe attempt are never freed, resulting in kmemleak reports showing multiple leaked netdev-related allocations all traced back to dpaa2_caam_probe().

Fix this by preserving the CPU mask of allocated netdevs during setup and using it for cleanup in dpaa2_dpseci_free(). This approach ensures that only the CPUs that actually had netdevs allocated will be cleaned up, avoiding potential issues with CPU hotplug scenarios.

Fixes 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2")

---
v2:
- fix the build error with CPUMASK_OFFSTACK disabled
- instead of the movement of free_dpaa2_pcpu_netdev, implement it directly in dpaa2_dpseci_free
v1: https://lore.kernel.org/all/[email protected]/

 drivers/crypto/caam/caamalg_qi2.c | 27 +++++++++++++++------------
 drivers/crypto/caam/caamalg_qi2.h |  2 ++
 2 files changed, 17 insertions(+), 12 deletions(-)
drivers/crypto/caam/caamalg_qi2.c +15 -12
index 107ccb2ade42..c6117c23eb25 100644
@@ -4814,7 +4814,8 @@ static void dpaa2_dpseci_free(struct dpaa2_caam_priv *priv)
 {
 	struct device *dev = priv->dev;
 	struct fsl_mc_device *ls_dev = to_fsl_mc_device(dev);
-	int err;
+	struct dpaa2_caam_priv_per_cpu *ppriv;
+	int i, err;
 
 	if (DPSECI_VER(priv->major_ver, priv->minor_ver) > DPSECI_VER(5, 3)) {
 		err = dpseci_reset(priv->mc_io, 0, ls_dev->mc_handle);
@@ -4822,6 +4823,12 @@ static void dpaa2_dpseci_free(struct dpaa2_caam_priv *priv)
 			dev_err(dev, "dpseci_reset() failed\n");
 	}
 
+	for_each_cpu(i, priv->clean_mask) {
+		ppriv = per_cpu_ptr(priv->ppriv, i);
+		free_netdev(ppriv->net_dev);
+	}
+	free_cpumask_var(priv->clean_mask);
+
 	dpaa2_dpseci_congestion_free(priv);
 	dpseci_close(priv->mc_io, 0, ls_dev->mc_handle);
 }
@@ -5007,16 +5014,15 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 	struct device *dev = &ls_dev->dev;
 	struct dpaa2_caam_priv *priv;
 	struct dpaa2_caam_priv_per_cpu *ppriv;
-	cpumask_var_t clean_mask;
 	int err, cpu;
 	u8 i;
 
 	err = -ENOMEM;
-	if (!zalloc_cpumask_var(&clean_mask, GFP_KERNEL))
-		goto err_cpumask;
-
 	priv = dev_get_drvdata(dev);
 
+	if (!zalloc_cpumask_var(&priv->clean_mask, GFP_KERNEL))
+		goto err_cpumask;
+
 	priv->dev = dev;
 	priv->dpsec_id = ls_dev->obj_desc.id;
 
@@ -5118,7 +5124,7 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 			err = -ENOMEM;
 			goto err_alloc_netdev;
 		}
-		cpumask_set_cpu(cpu, clean_mask);
+		cpumask_set_cpu(cpu, priv->clean_mask);
 		ppriv->net_dev->dev = *dev;
 
 		netif_napi_add_tx_weight(ppriv->net_dev, &ppriv->napi,
@@ -5126,18 +5132,16 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
 					 DPAA2_CAAM_NAPI_WEIGHT);
 	}
 
-	err = 0;
-	goto free_cpumask;
+	return 0;
 
 err_alloc_netdev:
-	free_dpaa2_pcpu_netdev(priv, clean_mask);
+	free_dpaa2_pcpu_netdev(priv, priv->clean_mask);
 err_get_rx_queue:
 	dpaa2_dpseci_congestion_free(priv);
 err_get_vers:
 	dpseci_close(priv->mc_io, 0, ls_dev->mc_handle);
 err_open:
-free_cpumask:
-	free_cpumask_var(clean_mask);
+	free_cpumask_var(priv->clean_mask);
 err_cpumask:
 	return err;
 }
@@ -5182,7 +5186,6 @@ static int __cold dpaa2_dpseci_disable(struct dpaa2_caam_priv *priv)
 		ppriv = per_cpu_ptr(priv->ppriv, i);
 		napi_disable(&ppriv->napi);
 		netif_napi_del(&ppriv->napi);
-		free_netdev(ppriv->net_dev);
 	}
 
 	return 0;
drivers/crypto/caam/caamalg_qi2.h +2 -0
index 61d1219a202f..8e65b4b28c7b 100644
@@ -42,6 +42,7 @@
  * @mc_io: pointer to MC portal's I/O object
  * @domain: IOMMU domain
  * @ppriv: per CPU pointers to privata data
+ * @clean_mask: CPU mask of CPUs that have allocated netdevs
  */
 struct dpaa2_caam_priv {
 	int dpsec_id;
@@ -65,6 +66,7 @@ struct dpaa2_caam_priv {
 
 	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
 	struct dentry *dfs_root;
+	cpumask_var_t clean_mask;
 };
 
 /**
Signature · 1 line
2.52.0
#1 lore raw

Gentle ping, any comments on this patch?

Thanks,
Jianpeng

Quoted 136 lines · Jianpeng Chang

When commit 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2") converted embedded net_device to dynamically allocated pointers, it added cleanup in dpaa2_dpseci_disable() but missed adding cleanup in dpaa2_dpseci_free() for error paths.

This causes memory leaks when dpaa2_dpseci_dpio_setup() fails during probe due to DPIO devices not being ready yet. The kernel's deferred probe mechanism handles the retry successfully, but the netdevs allocated during the failed probe attempt are never freed, resulting in kmemleak reports showing multiple leaked netdev-related allocations all traced back to dpaa2_caam_probe().

Fix this by preserving the CPU mask of allocated netdevs during setup and using it for cleanup in dpaa2_dpseci_free(). This approach ensures that only the CPUs that actually had netdevs allocated will be cleaned up, avoiding potential issues with CPU hotplug scenarios.

Fixes 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2")

---
v2:
- fix the build error with CPUMASK_OFFSTACK disabled
- instead of the movement of free_dpaa2_pcpu_netdev, implement it directly in dpaa2_dpseci_free
v1: https://lore.kernel.org/all/[email protected]/

  drivers/crypto/caam/caamalg_qi2.c | 27 +++++++++++++++------------
  drivers/crypto/caam/caamalg_qi2.h |  2 ++
  2 files changed, 17 insertions(+), 12 deletions(-)
drivers/crypto/caam/caamalg_qi2.c +15 -12
index 107ccb2ade42..c6117c23eb25 100644
@@ -4814,7 +4814,8 @@ static void dpaa2_dpseci_free(struct dpaa2_caam_priv *priv)
  {
  	struct device *dev = priv->dev;
  	struct fsl_mc_device *ls_dev = to_fsl_mc_device(dev);
-	int err;
+	struct dpaa2_caam_priv_per_cpu *ppriv;
+	int i, err;
 
  	if (DPSECI_VER(priv->major_ver, priv->minor_ver) > DPSECI_VER(5, 3)) {
  		err = dpseci_reset(priv->mc_io, 0, ls_dev->mc_handle);
@@ -4822,6 +4823,12 @@ static void dpaa2_dpseci_free(struct dpaa2_caam_priv *priv)
  			dev_err(dev, "dpseci_reset() failed\n");
  	}
 
+	for_each_cpu(i, priv->clean_mask) {
+		ppriv = per_cpu_ptr(priv->ppriv, i);
+		free_netdev(ppriv->net_dev);
+	}
+	free_cpumask_var(priv->clean_mask);
+
  	dpaa2_dpseci_congestion_free(priv);
  	dpseci_close(priv->mc_io, 0, ls_dev->mc_handle);
  }
@@ -5007,16 +5014,15 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
  	struct device *dev = &ls_dev->dev;
  	struct dpaa2_caam_priv *priv;
  	struct dpaa2_caam_priv_per_cpu *ppriv;
-	cpumask_var_t clean_mask;
  	int err, cpu;
  	u8 i;
 
  	err = -ENOMEM;
-	if (!zalloc_cpumask_var(&clean_mask, GFP_KERNEL))
-		goto err_cpumask;
-
  	priv = dev_get_drvdata(dev);
 
+	if (!zalloc_cpumask_var(&priv->clean_mask, GFP_KERNEL))
+		goto err_cpumask;
+
  	priv->dev = dev;
  	priv->dpsec_id = ls_dev->obj_desc.id;
 
@@ -5118,7 +5124,7 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
  			err = -ENOMEM;
  			goto err_alloc_netdev;
  		}
-		cpumask_set_cpu(cpu, clean_mask);
+		cpumask_set_cpu(cpu, priv->clean_mask);
  		ppriv->net_dev->dev = *dev;
 
  		netif_napi_add_tx_weight(ppriv->net_dev, &ppriv->napi,
@@ -5126,18 +5132,16 @@ static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
  					 DPAA2_CAAM_NAPI_WEIGHT);
  	}
 
-	err = 0;
-	goto free_cpumask;
+	return 0;
 
  err_alloc_netdev:
-	free_dpaa2_pcpu_netdev(priv, clean_mask);
+	free_dpaa2_pcpu_netdev(priv, priv->clean_mask);
  err_get_rx_queue:
  	dpaa2_dpseci_congestion_free(priv);
  err_get_vers:
  	dpseci_close(priv->mc_io, 0, ls_dev->mc_handle);
  err_open:
-free_cpumask:
-	free_cpumask_var(clean_mask);
+	free_cpumask_var(priv->clean_mask);
  err_cpumask:
  	return err;
  }
@@ -5182,7 +5186,6 @@ static int __cold dpaa2_dpseci_disable(struct dpaa2_caam_priv *priv)
  		ppriv = per_cpu_ptr(priv->ppriv, i);
  		napi_disable(&ppriv->napi);
  		netif_napi_del(&ppriv->napi);
-		free_netdev(ppriv->net_dev);
  	}
 
  	return 0;
drivers/crypto/caam/caamalg_qi2.h +2 -0
index 61d1219a202f..8e65b4b28c7b 100644
@@ -42,6 +42,7 @@
   * @mc_io: pointer to MC portal's I/O object
   * @domain: IOMMU domain
   * @ppriv: per CPU pointers to privata data
+ * @clean_mask: CPU mask of CPUs that have allocated netdevs
   */
  struct dpaa2_caam_priv {
  	int dpsec_id;
@@ -65,6 +66,7 @@ struct dpaa2_caam_priv {
 
  	struct dpaa2_caam_priv_per_cpu __percpu *ppriv;
  	struct dentry *dfs_root;
+	cpumask_var_t clean_mask;
  };
 
  /**
Reviewed-by on v2 1/1
#2 lore raw
In reply to Jianpeng Chang
Quoted 12 lines · Jianpeng Chang

When commit 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2") converted embedded net_device to dynamically allocated pointers, it added cleanup in dpaa2_dpseci_disable() but missed adding cleanup in dpaa2_dpseci_free() for error paths.

This causes memory leaks when dpaa2_dpseci_dpio_setup() fails during probe due to DPIO devices not being ready yet. The kernel's deferred probe mechanism handles the retry successfully, but the netdevs allocated during the failed probe attempt are never freed, resulting in kmemleak reports showing multiple leaked netdev-related allocations all traced back to dpaa2_caam_probe().

Quoted 7 lines

Fix this by preserving the CPU mask of allocated netdevs during setup and using it for cleanup in dpaa2_dpseci_free(). This approach ensures that only the CPUs that actually had netdevs allocated will be cleaned up, avoiding potential issues with CPU hotplug scenarios.

Fixes 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2")
maintainer
#3 lore raw
In reply to Jianpeng Chang
Quoted 26 lines · Jianpeng Chang

When commit 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2") converted embedded net_device to dynamically allocated pointers, it added cleanup in dpaa2_dpseci_disable() but missed adding cleanup in dpaa2_dpseci_free() for error paths.

This causes memory leaks when dpaa2_dpseci_dpio_setup() fails during probe due to DPIO devices not being ready yet. The kernel's deferred probe mechanism handles the retry successfully, but the netdevs allocated during the failed probe attempt are never freed, resulting in kmemleak reports showing multiple leaked netdev-related allocations all traced back to dpaa2_caam_probe().

Fix this by preserving the CPU mask of allocated netdevs during setup and using it for cleanup in dpaa2_dpseci_free(). This approach ensures that only the CPUs that actually had netdevs allocated will be cleaned up, avoiding potential issues with CPU hotplug scenarios.

Fixes 0e1a4d427f58 ("crypto: caam: Unembed net_dev structure in dpaa2")

---
v2:
- fix the build error with CPUMASK_OFFSTACK disabled
- instead of the movement of free_dpaa2_pcpu_netdev, implement it directly in dpaa2_dpseci_free
v1: https://lore.kernel.org/all/[email protected]/

Quoted 3 lines
 drivers/crypto/caam/caamalg_qi2.c | 27 +++++++++++++++------------
 drivers/crypto/caam/caamalg_qi2.h |  2 ++
 2 files changed, 17 insertions(+), 12 deletions(-)

Patch applied. Thanks.

Signature · 3 lines
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt