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;
>
>