Hi Ira, Thanks for the detailed explanation.
I see now that nd_pfn_validate() already handles the NULL pfn_sb case, and the error path correctly releases the device. You are right that the patch is unnecessary. I will drop this patch. Thanks for your time reviewing it. Best regards, Zhaoyang 原始邮件 发件人：Ira Weiny <[email protected]> 发件时间：2026年1月27日 04:27 收件人：Zhaoyang Yu <[email protected]>, dan.j.williams <[email protected]> 抄送：vishal.l.verma <[email protected]>, dave.jiang <[email protected]>, ira.weiny <[email protected]>, nvdimm <[email protected]>, linux-kernel <[email protected]>, gszhai <[email protected]>, Zhaoyang Yu <[email protected]> 主题：Re: [PATCH] nvdimm: Add check for devm_kmalloc() and fix NULLpointer dereference in nd_pfn_probe() and nd_dax_probe() Zhaoyang Yu wrote: > The devm_kmalloc() function may return NULL when memory allocation fails. > In nd_pfn_probe() and nd_dax_probe(), the return values of devm_kmalloc() > are not checked. If pfn_sb is NULL, it will cause a NULL pointer > dereference in the subsequent calls to nd_pfn_validate(). > > Additionally, if the allocation fails, the devices initialized by > nd_pfn_devinit() or nd_dax_devinit() are not properly released, leading > to memory leaks. > > Fix this by checking the return value of devm_kmalloc() in both functions. > If the allocation fails, use put_device() to release the initialized device > and return -ENOMEM. > > Signed-off-by: Zhaoyang Yu <[email protected]> > --- > drivers/nvdimm/dax_devs.c | 4 ++++ > drivers/nvdimm/pfn_devs.c | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/nvdimm/dax_devs.c b/drivers/nvdimm/dax_devs.c > index ba4c409ede65..aa51a9022d12 100644 > --- a/drivers/nvdimm/dax_devs.c > +++ b/drivers/nvdimm/dax_devs.c > @@ -111,6 +111,10 @@ int nd_dax_probe(struct device *dev, struct >nd_namespace_common *ndns) > return -ENOMEM; > } > pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL); > + if (!pfn_sb) { > + put_device(dax_dev); > + return -ENOMEM; > + } Sorry this is a NAK. While I don't like the implicit nature of the check... This is not needed. The validity of pfn_sb is checked in nd_pfn_validate() It is unfortunate that the errno reported in that case is ENODEV rather than ENOMEM... But I would not change that now. > nd_pfn = &nd_dax->nd_pfn; > nd_pfn->pfn_sb = pfn_sb; > rc = nd_pfn_validate(nd_pfn, DAX_SIG); > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index 42b172fc5576..6a69d8bfeb7c 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -635,6 +635,10 @@ int nd_pfn_probe(struct device *dev, struct >nd_namespace_common *ndns) > if (!pfn_dev) > return -ENOMEM; > pfn_sb = devm_kmalloc(dev, sizeof(*pfn_sb), GFP_KERNEL); > + if (!pfn_sb) { > + put_device(pfn_dev); > + return -ENOMEM; > + } > nd_pfn = to_nd_pfn(pfn_dev); > nd_pfn->pfn_sb = pfn_sb; > rc = nd_pfn_validate(nd_pfn, PFN_SIG); Same issue here. Ira
