linux-kernel - Re: [PATCH v2] nvdimm: ndtest: Return -ENOMEM if devm_kcalloc() fails in ndtest

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>] [day] [month] [year] [list]

Message-ID: <0116557c-3b60-441e-8976-ebce1a658a01@intel.com>
Date: Wed, 24 Sep 2025 08:34:30 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Guangshuo Li <lgs201920130244@...il.com>,
 Alison Schofield <alison.schofield@...el.com>
Cc: Dan Williams <dan.j.williams@...el.com>,
 Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
 Santosh Sivaraj <santosh@...six.org>, nvdimm@...ts.linux.dev,
 linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2] nvdimm: ndtest: Return -ENOMEM if devm_kcalloc() fails
 in ndtest_probe()



On 9/24/25 12:42 AM, Guangshuo Li wrote:
> Hi Alison, Dave, and all,
> 
> Thanks for the feedback. I’ve adopted your suggestions. Below is what I plan to take in v3.
> 
> -       p->dcr_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                 sizeof(dma_addr_t), GFP_KERNEL);
> -       p->label_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                   sizeof(dma_addr_t), GFP_KERNEL);
> -       p->dimm_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> -                                  sizeof(dma_addr_t), GFP_KERNEL);
> 
> +       p->dcr_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                 sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->dcr_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
> +
> +       p->label_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                   sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->label_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }
> +
> +       p->dimm_dma = devm_kcalloc(&p->pdev.dev <http://pdev.dev>, NUM_DCR,
> +                                  sizeof(dma_addr_t), GFP_KERNEL);
> +       if (!p->dimm_dma) {
> +               rc = -ENOMEM;
> +               goto err;
> +       }

You'll need to create new goto labels because you'll have to free previously allocated memory in the error path. Diff below is uncompiled and untested.

diff --git a/tools/testing/nvdimm/test/ndtest.c b/tools/testing/nvdimm/test/ndtest.c
index 68a064ce598c..49d326819ea9 100644
--- a/tools/testing/nvdimm/test/ndtest.c
+++ b/tools/testing/nvdimm/test/ndtest.c
@@ -841,6 +841,7 @@ static void ndtest_remove(struct platform_device *pdev)

 static int ndtest_probe(struct platform_device *pdev)
 {
+       struct device *dev = &pdev->dev;
        struct ndtest_priv *p;
        int rc;

@@ -848,12 +849,23 @@ static int ndtest_probe(struct platform_device *pdev)
        if (ndtest_bus_register(p))
                return -ENOMEM;

-       p->dcr_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                sizeof(dma_addr_t), GFP_KERNEL);
-       p->label_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                  sizeof(dma_addr_t), GFP_KERNEL);
-       p->dimm_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
-                                 sizeof(dma_addr_t), GFP_KERNEL);
+       p->dcr_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t), GFP_KERNEL);
+       if (!p->dcr_dma)
+               return -ENOMEM;
+
+       p->label_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t),
+                                   GFP_KERNEL);
+       if (!p->label_dma) {
+               rc = -ENOMEM;
+               goto err_label_dma;
+       }
+
+       p->dimm_dma = devm_kcalloc(dev, NUM_DCR, sizeof(dma_addr_t),
+                                  GFP_KERNEL);
+       if (!p->dimm_dma) {
+               rc = -ENOMEM;
+               goto err_dimm_dma;
+       }

        rc = ndtest_nvdimm_init(p);
        if (rc)
@@ -863,7 +875,7 @@ static int ndtest_probe(struct platform_device *pdev)
        if (rc)
                goto err;

-       rc = devm_add_action_or_reset(&pdev->dev, put_dimms, p);
+       rc = devm_add_action_or_reset(dev, put_dimms, p);
        if (rc)
                goto err;
@@ -872,6 +884,11 @@ static int ndtest_probe(struct platform_device *pdev)
        return 0;

 err:
+       devm_kfree(dev, p->dimm_dma);
+err_dimm_dma:
+       devm_kfree(dev, p->label_dma);
+err_label_dma:
+       devm_kfree(dev, p->dcr_dma);
        pr_err("%s:%d Failed nvdimm init\n", __func__, __LINE__);
        return rc;
 }

> 
> If this looks good, I’ll send v3 accordingly. Also, if you’re comfortable with the changes, may I add your Reviewed-by tags?

Please don't add review tags until they are given.

> 
> Best regards,
> Guangshuo