| Message ID | 20230817114103.754977-1-konstantin.meskhidze@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 08ca6906a4b7e48f8e93b7c1f49a742a415be6d5 |
| Headers | show |
| Series | drivers: nvdimm: fix dereference after free | expand |
Konstantin Meskhidze <konstantin.meskhidze@huawei.com> writes: > 'nd_pmu->pmu.attr_groups' is dereferenced in function > 'nvdimm_pmu_free_hotplug_memory' call after it has been freed. Because in > function 'nvdimm_pmu_free_hotplug_memory' memory pointed by the fields of > 'nd_pmu->pmu.attr_groups' is deallocated it is necessary to call 'kfree' > after 'nvdimm_pmu_free_hotplug_memory'. > > Co-developed-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> > Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> > --- > drivers/nvdimm/nd_perf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvdimm/nd_perf.c b/drivers/nvdimm/nd_perf.c > index 14881c4e0..2b6dc80d8 100644 > --- a/drivers/nvdimm/nd_perf.c > +++ b/drivers/nvdimm/nd_perf.c > @@ -307,10 +307,10 @@ int register_nvdimm_pmu(struct nvdimm_pmu *nd_pmu, struct platform_device *pdev) > } > > rc = perf_pmu_register(&nd_pmu->pmu, nd_pmu->pmu.name, -1); > if (rc) { > - kfree(nd_pmu->pmu.attr_groups); > nvdimm_pmu_free_hotplug_memory(nd_pmu); > + kfree(nd_pmu->pmu.attr_groups); > return rc; > } > > pr_info("%s NVDIMM performance monitor support registered\n", Reviewed-by: Jeff Moyer <jmoyer@redhat.com>
On 8/17/23 04:41, Konstantin Meskhidze wrote: > 'nd_pmu->pmu.attr_groups' is dereferenced in function > 'nvdimm_pmu_free_hotplug_memory' call after it has been freed. Because in > function 'nvdimm_pmu_free_hotplug_memory' memory pointed by the fields of > 'nd_pmu->pmu.attr_groups' is deallocated it is necessary to call 'kfree' > after 'nvdimm_pmu_free_hotplug_memory'. > > Co-developed-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> > Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> LGTM Does this need a Fixes tag? > --- > drivers/nvdimm/nd_perf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvdimm/nd_perf.c b/drivers/nvdimm/nd_perf.c > index 14881c4e0..2b6dc80d8 100644 > --- a/drivers/nvdimm/nd_perf.c > +++ b/drivers/nvdimm/nd_perf.c > @@ -307,10 +307,10 @@ int register_nvdimm_pmu(struct nvdimm_pmu *nd_pmu, struct platform_device *pdev) > } > > rc = perf_pmu_register(&nd_pmu->pmu, nd_pmu->pmu.name, -1); > if (rc) { > - kfree(nd_pmu->pmu.attr_groups); > nvdimm_pmu_free_hotplug_memory(nd_pmu); > + kfree(nd_pmu->pmu.attr_groups); > return rc; > } > > pr_info("%s NVDIMM performance monitor support registered\n",
[ add Kajol ] Konstantin Meskhidze wrote: > 'nd_pmu->pmu.attr_groups' is dereferenced in function > 'nvdimm_pmu_free_hotplug_memory' call after it has been freed. Because in > function 'nvdimm_pmu_free_hotplug_memory' memory pointed by the fields of > 'nd_pmu->pmu.attr_groups' is deallocated it is necessary to call 'kfree' > after 'nvdimm_pmu_free_hotplug_memory'. Another one that would be fixed by static attribute groups. I do think we should move forward with these fixes as is for ease of backport, but long term this dynamically allocated attribute groups approach needs to be jettisoned. ...unless I am missing a concrete reason it needs to remain dynamic?
On 8/17/23 08:45, Dave Jiang wrote: > > > On 8/17/23 04:41, Konstantin Meskhidze wrote: >> 'nd_pmu->pmu.attr_groups' is dereferenced in function >> 'nvdimm_pmu_free_hotplug_memory' call after it has been freed. Because in >> function 'nvdimm_pmu_free_hotplug_memory' memory pointed by the fields of >> 'nd_pmu->pmu.attr_groups' is deallocated it is necessary to call 'kfree' >> after 'nvdimm_pmu_free_hotplug_memory'. >> >> Co-developed-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> >> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> > > LGTM > > Does this need a Fixes tag? Applied. Modified subject to "nvdimm: Fix dereference after free in register_nvdimm_pmu()" Also added Fixes tag: Fixes: 0fab1ba6ad6b ("drivers/nvdimm: Add perf interface to expose nvdimm performance stats") > >> --- >> drivers/nvdimm/nd_perf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/nvdimm/nd_perf.c b/drivers/nvdimm/nd_perf.c >> index 14881c4e0..2b6dc80d8 100644 >> --- a/drivers/nvdimm/nd_perf.c >> +++ b/drivers/nvdimm/nd_perf.c >> @@ -307,10 +307,10 @@ int register_nvdimm_pmu(struct nvdimm_pmu >> *nd_pmu, struct platform_device *pdev) >> } >> rc = perf_pmu_register(&nd_pmu->pmu, nd_pmu->pmu.name, -1); >> if (rc) { >> - kfree(nd_pmu->pmu.attr_groups); >> nvdimm_pmu_free_hotplug_memory(nd_pmu); >> + kfree(nd_pmu->pmu.attr_groups); >> return rc; >> } >> pr_info("%s NVDIMM performance monitor support registered\n", >
diff --git a/drivers/nvdimm/nd_perf.c b/drivers/nvdimm/nd_perf.c index 14881c4e0..2b6dc80d8 100644 --- a/drivers/nvdimm/nd_perf.c +++ b/drivers/nvdimm/nd_perf.c @@ -307,10 +307,10 @@ int register_nvdimm_pmu(struct nvdimm_pmu *nd_pmu, struct platform_device *pdev) } rc = perf_pmu_register(&nd_pmu->pmu, nd_pmu->pmu.name, -1); if (rc) { - kfree(nd_pmu->pmu.attr_groups); nvdimm_pmu_free_hotplug_memory(nd_pmu); + kfree(nd_pmu->pmu.attr_groups); return rc; } pr_info("%s NVDIMM performance monitor support registered\n",
'nd_pmu->pmu.attr_groups' is dereferenced in function 'nvdimm_pmu_free_hotplug_memory' call after it has been freed. Because in function 'nvdimm_pmu_free_hotplug_memory' memory pointed by the fields of 'nd_pmu->pmu.attr_groups' is deallocated it is necessary to call 'kfree' after 'nvdimm_pmu_free_hotplug_memory'. Co-developed-by: Ivanov Mikhail <ivanov.mikhail1@huawei-partners.com> Signed-off-by: Konstantin Meskhidze <konstantin.meskhidze@huawei.com> --- drivers/nvdimm/nd_perf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)