| Message ID | 20230619033623.11044-1-jiasheng@iscas.ac.cn (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Ira Weiny |
| Headers | show |
| Series | libnvdimm/of_pmem: Add check and kfree for kstrdup | expand |
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 <jiasheng@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. Ira > kfree(priv); > return -ENODEV; > } > -- > 2.25.1 >
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 <jiasheng@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. Ira
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); kfree(priv); return -ENODEV; }
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 <jiasheng@iscas.ac.cn> --- drivers/nvdimm/of_pmem.c | 6 ++++++ 1 file changed, 6 insertions(+)