On Wed, Jun 21, 2023 at 00:04:36 +0800, Ira Weiny wrote: > Ira Weiny wrote: >> Jiasheng Jiang wrote: >> > Add check for the return value of kstrdup() and return the error >> > if it fails in order to avoid NULL pointer dereference. >> > Moreover, use kfree() in the later error handling in order to avoid >> > memory leak. >> > >> > Fixes: 49bddc73d15c ("libnvdimm/of_pmem: Provide a unique name for bus >> > provider") >> > Signed-off-by: Jiasheng Jiang <jiash...@iscas.ac.cn> >> > --- >> > drivers/nvdimm/of_pmem.c | 6 ++++++ >> > 1 file changed, 6 insertions(+) >> > >> > diff --git a/drivers/nvdimm/of_pmem.c b/drivers/nvdimm/of_pmem.c >> > index 10dbdcdfb9ce..fe6edb7e6631 100644 >> > --- a/drivers/nvdimm/of_pmem.c >> > +++ b/drivers/nvdimm/of_pmem.c >> > @@ -31,11 +31,17 @@ static int of_pmem_region_probe(struct platform_device >> > *pdev) >> > return -ENOMEM; >> > >> > priv->bus_desc.provider_name = kstrdup(pdev->name, GFP_KERNEL); >> > + if (!priv->bus_desc.provider_name) { >> > + kfree(priv); >> > + return -ENOMEM; >> > + } >> > + >> > priv->bus_desc.module = THIS_MODULE; >> > priv->bus_desc.of_node = np; >> > >> > priv->bus = bus = nvdimm_bus_register(&pdev->dev, &priv->bus_desc); >> > if (!bus) { >> > + kfree(priv->bus_desc.provider_name); >> >> Nice catch! >> >> However, this free needs to happen in of_pmem_region_remove() as well. > > Looks like the mail from my phone had html in it. Sorry for that. > > This would be better with devm_kstrdup() and then we don't have to worry > about the kfree at all.
Looks good. I have submitted a new patch "libnvdimm/of_pmem: Replace kstrdup with devm_kstrdup and add check". Since the titie has been modified, I did not submitted a v2. - Jiasheng Jiang