On Tue, Sep 23, 2025 at 09:38:33AM -0700, Dave Jiang wrote:
> 
> 
> On 9/23/25 5:59 AM, Guangshuo Li wrote:
> > devm_kcalloc() may fail. ndtest_probe() allocates three DMA address
> > arrays (dcr_dma, label_dma, dimm_dma) and later unconditionally uses
> > them in ndtest_nvdimm_init(), which can lead to a NULL pointer
> > dereference under low-memory conditions.
> > 
> > Check all three allocations and return -ENOMEM if any allocation fails.
> > Do not emit an extra error message since the allocator already warns on
> > allocation failure.
> > 
> > Fixes: 9399ab61ad82 ("ndtest: Add dimms to the two buses")
> > Cc: [email protected]
> > Signed-off-by: Guangshuo Li <[email protected]>
> > ---
> > Changes in v2:
> > - Drop pr_err() on allocation failure; only NULL-check and return -ENOMEM.
> > - No other changes.
> > ---
> >  tools/testing/nvdimm/test/ndtest.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/testing/nvdimm/test/ndtest.c 
> > b/tools/testing/nvdimm/test/ndtest.c
> > index 68a064ce598c..abdbe0c1cb63 100644
> > --- a/tools/testing/nvdimm/test/ndtest.c
> > +++ b/tools/testing/nvdimm/test/ndtest.c
> > @@ -855,6 +855,9 @@ static int ndtest_probe(struct platform_device *pdev)
> >     p->dimm_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
> >                               sizeof(dma_addr_t), GFP_KERNEL);
> >  
> > +   if (!p->dcr_dma || !p->label_dma || !p->dimm_dma)
> > +           return -ENOMEM;
> 
> Why not just check as it allocates instead of doing it all at the end? If an 
> allocation failed, no need to attempt to allocate more (which probably leads 
> to more failures).

Following on Dave's feedback and looking at the function as a whole,
it does have a pattern that this patch can replicate:

It does this now:
        rc = do_something();
        if (rc)
                goto err;

So, continue that pattern with the added NULL checks:

        p->dcr_dma = devm_kcalloc(&p->pdev.dev, NUM_DCR,
                                  sizeof(dma_addr_t), GFP_KERNEL);
        if (!p->dcr_dma) {
                rc = -ENOMEM;
                goto err;
        }
and repeat above for all 3 allocs.

And, maybe even change that first ndtest_bus_register() failure
to follow the same pattern.

--Alison

> 
> DJ
> 
> > +
> >     rc = ndtest_nvdimm_init(p);
> >     if (rc)
> >             goto err;
> 
> 

Reply via email to