crypto: caam: fix netdev memory leak in dpaa2_caam_probe

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hi Jianpeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on herbert-cryptodev-2.6/master]
[also build test ERROR on herbert-crypto-2.6/master linus/master v6.19-rc5 next-20260115] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jianpeng-Chang/crypto-caam-fix-netdev-memory-leak-in-dpaa2_caam_probe/20260116-094800
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20260116014455.2575351-1-jianpeng.chang.cn%40windriver.com
patch subject: [PATCH] crypto: caam: fix netdev memory leak in dpaa2_caam_probe
config: x86_64-buildonly-randconfig-004-20260116 (https://download.01.org/0day-ci/archive/20260116/[email protected]/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260116/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

Quoted 1 line

Quoted 1 line

drivers/crypto/caam/caamalg_qi2.c:5132:19: error: array type 'cpumask_var_t' (aka 'struct cpumask[1]') is not assignable

     5132 |         priv->clean_mask = clean_mask;
          |         ~~~~~~~~~~~~~~~~ ^

1 error generated.

vim +5132 drivers/crypto/caam/caamalg_qi2.c

   5007
   5008  static int __cold dpaa2_dpseci_setup(struct fsl_mc_device *ls_dev)
   5009  {
   5010          struct device *dev = &ls_dev->dev;
   5011          struct dpaa2_caam_priv *priv;
   5012          struct dpaa2_caam_priv_per_cpu *ppriv;
   5013          cpumask_var_t clean_mask;
   5014          int err, cpu;
   5015          u8 i;
   5016
   5017          err = -ENOMEM;
   5018          if (!zalloc_cpumask_var(&clean_mask, GFP_KERNEL))
   5019                  goto err_cpumask;
   5020
   5021          priv = dev_get_drvdata(dev);
   5022
   5023          priv->dev = dev;
   5024          priv->dpsec_id = ls_dev->obj_desc.id;
   5025
   5026          /* Get a handle for the DPSECI this interface is associate with */
   5027          err = dpseci_open(priv->mc_io, 0, priv->dpsec_id, &ls_dev->mc_handle);
   5028          if (err) {
   5029                  dev_err(dev, "dpseci_open() failed: %d\n", err);
   5030                  goto err_open;
   5031          }
   5032
   5033          err = dpseci_get_api_version(priv->mc_io, 0, &priv->major_ver,
   5034                                       &priv->minor_ver);
   5035          if (err) {
   5036                  dev_err(dev, "dpseci_get_api_version() failed\n");
   5037                  goto err_get_vers;
   5038          }
   5039
   5040          dev_info(dev, "dpseci v%d.%d\n", priv->major_ver, priv->minor_ver);
   5041
   5042          if (DPSECI_VER(priv->major_ver, priv->minor_ver) > DPSECI_VER(5, 3)) {
   5043                  err = dpseci_reset(priv->mc_io, 0, ls_dev->mc_handle);
   5044                  if (err) {
   5045                          dev_err(dev, "dpseci_reset() failed\n");
   5046                          goto err_get_vers;
   5047                  }
   5048          }
   5049
   5050          err = dpseci_get_attributes(priv->mc_io, 0, ls_dev->mc_handle,
   5051                                      &priv->dpseci_attr);
   5052          if (err) {
   5053                  dev_err(dev, "dpseci_get_attributes() failed\n");
   5054                  goto err_get_vers;
   5055          }
   5056
   5057          err = dpseci_get_sec_attr(priv->mc_io, 0, ls_dev->mc_handle,
   5058                                    &priv->sec_attr);
   5059          if (err) {
   5060                  dev_err(dev, "dpseci_get_sec_attr() failed\n");
   5061                  goto err_get_vers;
   5062          }
   5063
   5064          err = dpaa2_dpseci_congestion_setup(priv, ls_dev->mc_handle);
   5065          if (err) {
   5066                  dev_err(dev, "setup_congestion() failed\n");
   5067                  goto err_get_vers;
   5068          }
   5069
   5070          priv->num_pairs = min(priv->dpseci_attr.num_rx_queues,
   5071                                priv->dpseci_attr.num_tx_queues);
   5072          if (priv->num_pairs > num_online_cpus()) {
   5073                  dev_warn(dev, "%d queues won't be used\n",
   5074                           priv->num_pairs - num_online_cpus());
   5075                  priv->num_pairs = num_online_cpus();
   5076          }
   5077
   5078          for (i = 0; i < priv->dpseci_attr.num_rx_queues; i++) {
   5079                  err = dpseci_get_rx_queue(priv->mc_io, 0, ls_dev->mc_handle, i,
   5080                                            &priv->rx_queue_attr[i]);
   5081                  if (err) {
   5082                          dev_err(dev, "dpseci_get_rx_queue() failed\n");
   5083                          goto err_get_rx_queue;
   5084                  }
   5085          }
   5086
   5087          for (i = 0; i < priv->dpseci_attr.num_tx_queues; i++) {
   5088                  err = dpseci_get_tx_queue(priv->mc_io, 0, ls_dev->mc_handle, i,
   5089                                            &priv->tx_queue_attr[i]);
   5090                  if (err) {
   5091                          dev_err(dev, "dpseci_get_tx_queue() failed\n");
   5092                          goto err_get_rx_queue;
   5093                  }
   5094          }
   5095
   5096          i = 0;
   5097          for_each_online_cpu(cpu) {
   5098                  u8 j;
   5099
   5100                  j = i % priv->num_pairs;
   5101
   5102                  ppriv = per_cpu_ptr(priv->ppriv, cpu);
   5103                  ppriv->req_fqid = priv->tx_queue_attr[j].fqid;
   5104
   5105                  /*
   5106                   * Allow all cores to enqueue, while only some of them
   5107                   * will take part in dequeuing.
   5108                   */
   5109                  if (++i > priv->num_pairs)
   5110                          continue;
   5111
   5112                  ppriv->rsp_fqid = priv->rx_queue_attr[j].fqid;
   5113                  ppriv->prio = j;
   5114
   5115                  dev_dbg(dev, "pair %d: rx queue %d, tx queue %d\n", j,
   5116                          priv->rx_queue_attr[j].fqid,
   5117                          priv->tx_queue_attr[j].fqid);
   5118
   5119                  ppriv->net_dev = alloc_netdev_dummy(0);
   5120                  if (!ppriv->net_dev) {
   5121                          err = -ENOMEM;
   5122                          goto err_alloc_netdev;
   5123                  }
   5124                  cpumask_set_cpu(cpu, clean_mask);
   5125                  ppriv->net_dev->dev = *dev;
   5126
   5127                  netif_napi_add_tx_weight(ppriv->net_dev, &ppriv->napi,
   5128                                           dpaa2_dpseci_poll,
   5129                                           DPAA2_CAAM_NAPI_WEIGHT);
   5130          }
   5131

Quoted 1 line

5132 priv->clean_mask = clean_mask;

   5133          return 0;
   5134
   5135  err_alloc_netdev:
   5136          free_dpaa2_pcpu_netdev(priv, clean_mask);
   5137  err_get_rx_queue:
   5138          dpaa2_dpseci_congestion_free(priv);
   5139  err_get_vers:
   5140          dpseci_close(priv->mc_io, 0, ls_dev->mc_handle);
   5141  err_open:
   5142          free_cpumask_var(clean_mask);
   5143  err_cpumask:
   5144          return err;
   5145  }
   5146

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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")

Signed-off-by Jianpeng Chang <[email protected]>

---
 drivers/crypto/caam/caamalg_qi2.c | 31 ++++++++++++++++---------------
 drivers/crypto/caam/caamalg_qi2.h |  2 ++
 2 files changed, 18 insertions(+), 15 deletions(-)

drivers/crypto/caam/caamalg_qi2.c +0 -0

index 107ccb2ade42..a66c62174a0f 100644
@@ -4810,6 +4810,17 @@ static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv)
 	kfree(priv->cscn_mem);
 }
 

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

Quoted 32 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")

Signed-off-by Jianpeng Chang <[email protected]>

---
  drivers/crypto/caam/caamalg_qi2.c | 31 ++++++++++++++++---------------
  drivers/crypto/caam/caamalg_qi2.h |  2 ++
  2 files changed, 18 insertions(+), 15 deletions(-)

drivers/crypto/caam/caamalg_qi2.c +0 -0

index 107ccb2ade42..a66c62174a0f 100644
@@ -4810,6 +4810,17 @@ static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv)
       kfree(priv->cscn_mem);
  }
 

drivers/crypto/caam/caamalg_qi2.c static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv) · line 4822

Quoted 10 lines

drivers/crypto/caam/caamalg_qi2.c +10 -0

+static void free_dpaa2_pcpu_netdev(struct dpaa2_caam_priv *priv, const cpumask_t *cpus)
+{
+     struct dpaa2_caam_priv_per_cpu *ppriv;
+     int i;
+
+     for_each_cpu(i, cpus) {
+             ppriv = per_cpu_ptr(priv->ppriv, i);
+             free_netdev(ppriv->net_dev);
+     }
+}

Why is the function being moved here? Please keep code movement separate from functional changes, or at minimum explain why the move is necessary in the commit message.

Quoted 38 lines · Breno Leitao

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

Quoted 29 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")

Signed-off-by Jianpeng Chang <[email protected]>

---
  drivers/crypto/caam/caamalg_qi2.c | 31 ++++++++++++++++---------------
  drivers/crypto/caam/caamalg_qi2.h |  2 ++
  2 files changed, 18 insertions(+), 15 deletions(-)

drivers/crypto/caam/caamalg_qi2.c +0 -0

index 107ccb2ade42..a66c62174a0f 100644

drivers/crypto/caam/caamalg_qi2.c static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv) · line 4813

Quoted 4 lines

drivers/crypto/caam/caamalg_qi2.c +1 -0

@@ -4810,6 +4810,17 @@ static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv)
       kfree(priv->cscn_mem);
  }
 
+static void free_dpaa2_pcpu_netdev(struct dpaa2_caam_priv *priv, const cpumask_t *cpus)

c> +{

Quoted 12 lines

drivers/crypto/caam/caamalg_qi2.c static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv) · line 4822

Quoted 8 lines

drivers/crypto/caam/caamalg_qi2.c +8 -0

+     struct dpaa2_caam_priv_per_cpu *ppriv;
+     int i;
+
+     for_each_cpu(i, cpus) {
+             ppriv = per_cpu_ptr(priv->ppriv, i);
+             free_netdev(ppriv->net_dev);
+     }
+}

Why is the function being moved here? Please keep code movement separate from functional changes, or at minimum explain why the move is necessary in the commit message.

Thank you for the feedback.

I moved the function because I thought reusing existing code would be cleaner in dpaa2_dpseci_free. I will add the explain in commit message.

For future reference, what's the preferred approach when needing to reuse a simple function (4-line loop) defined later in the file - forward declaration, move it with a separate change or just implement directly?

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

Hello Jianpeng,

Quoted 51 lines · Jianpeng (CN)

Quoted 38 lines · Breno Leitao

CAUTION: This email comes from a non Wind River email account!
Do not click links or open attachments unless you recognize the sender and know the content is safe.

Quoted 29 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")

Signed-off-by Jianpeng Chang <[email protected]>

---
   drivers/crypto/caam/caamalg_qi2.c | 31 ++++++++++++++++---------------
   drivers/crypto/caam/caamalg_qi2.h |  2 ++
   2 files changed, 18 insertions(+), 15 deletions(-)

drivers/crypto/caam/caamalg_qi2.c +0 -0

index 107ccb2ade42..a66c62174a0f 100644

drivers/crypto/caam/caamalg_qi2.c static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv) · line 4813

Quoted 4 lines

drivers/crypto/caam/caamalg_qi2.c +1 -0

@@ -4810,6 +4810,17 @@ static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv)
        kfree(priv->cscn_mem);
   }
 
+static void free_dpaa2_pcpu_netdev(struct dpaa2_caam_priv *priv, const cpumask_t *cpus)

c> +{

Quoted 12 lines

drivers/crypto/caam/caamalg_qi2.c static void dpaa2_dpseci_congestion_free(struct dpaa2_caam_priv *priv) · line 4822

Quoted 8 lines

drivers/crypto/caam/caamalg_qi2.c +8 -0

+     struct dpaa2_caam_priv_per_cpu *ppriv;
+     int i;
+
+     for_each_cpu(i, cpus) {
+             ppriv = per_cpu_ptr(priv->ppriv, i);
+             free_netdev(ppriv->net_dev);
+     }
+}

Why is the function being moved here? Please keep code movement separate from functional changes, or at minimum explain why the move is necessary in the commit message.

Quoted 8 lines

Thank you for the feedback.

I moved the function because I thought reusing existing code would be cleaner in dpaa2_dpseci_free. I will add the explain in commit message.

For future reference, what's the preferred approach when needing to reuse a simple function (4-line loop) defined later in the file - forward declaration, move it with a separate change or just implement directly?

It is fine to implement directly, but, I am a bit confused with the solution, let me back up a bit.

First, it seems the problem is real and thanks for fixing it.

Regarding the solution, I am wondering if it is not simpler to iterate the priv->num_pairs and kfreeing them in dpaa2_dpseci_free(), similarly to dpaa2_dpseci_disable().