Messages in this thread | | | Date | Sun, 26 Feb 2023 12:46:12 -0800 | From | Alison Schofield <> | Subject | Re: [PATCH] nvdimm: check for null return of devm_kmalloc in nd_pfn_probe |
| |
On Sun, Feb 26, 2023 at 01:56:15PM +0800, Kang Chen wrote: > devm_kmalloc may fails, pfn_sb might be null and will cause > null pointer dereference later. > > Signed-off-by: Kang Chen <void0red@gmail.com> > --- > drivers/nvdimm/pfn_devs.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/nvdimm/pfn_devs.c b/drivers/nvdimm/pfn_devs.c > index af7d93015..d24fad175 100644 > --- a/drivers/nvdimm/pfn_devs.c > +++ b/drivers/nvdimm/pfn_devs.c > @@ -640,6 +640,8 @@ 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) > + return -ENOMEM; > nd_pfn = to_nd_pfn(pfn_dev); > nd_pfn->pfn_sb = pfn_sb; > rc = nd_pfn_validate(nd_pfn, PFN_SIG);
Hi Kang,
I too, think the code is clearer if the failure to alloc is addressed immediately. In this case, it seems we can't just return -ENOMEM. The original code is detecting that NULL pfn_sb in nd_pfn_validate(), and then doing this cleanup upon return:
if (rc < 0) { nd_detach_ndns(pfn_dev, &nd_pfn->ndns); put_device(pfn_dev);
Perhaps refactor a bit to go right to the cleanup, as opposed to calling nd_pfn_validate() when !pfn_sb.
Alison
> -- > 2.34.1 > >
| |