| Message ID | 20180227100231.22561-3-embedded24@evers-fischer.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | pci: endpoint: Fix double free in pci_epf_create() | expand |
On Tuesday 27 February 2018 03:32 PM, Rolf Evers-Fischer wrote: > From: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> > > 'put_device()' calls the relase function 'pci_epf_dev_release()', > which already frees 'epf->name' and 'epf'. > > Therefore we must not free them again after 'put_device()'. > > Fixes: 5e8cb4033807 ("PCI: endpoint: Add EP core layer to enable EP controller and EP functions") > > Signed-off-by: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> Acked-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/endpoint/pci-epf-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 1f2506f32bb9..1878a6776519 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -232,7 +232,7 @@ struct pci_epf *pci_epf_create(const char *name) > > put_dev: > put_device(dev); > - kfree(epf->name); > + return ERR_PTR(ret); > > free_epf: > kfree(epf); >
On Tue, Feb 27, 2018 at 11:02:30AM +0100, Rolf Evers-Fischer wrote: > From: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> > > 'put_device()' calls the relase function 'pci_epf_dev_release()', > which already frees 'epf->name' and 'epf'. > > Therefore we must not free them again after 'put_device()'. > > Fixes: 5e8cb4033807 ("PCI: endpoint: Add EP core layer to enable EP controller and EP functions") > > Signed-off-by: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> > --- > drivers/pci/endpoint/pci-epf-core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > index 1f2506f32bb9..1878a6776519 100644 > --- a/drivers/pci/endpoint/pci-epf-core.c > +++ b/drivers/pci/endpoint/pci-epf-core.c > @@ -232,7 +232,7 @@ struct pci_epf *pci_epf_create(const char *name) > > put_dev: > put_device(dev); > - kfree(epf->name); > + return ERR_PTR(ret); Another thing you could do, which would get rid of these multiple return statements (yes there is another one) would consist in removing the goto labels completely and handle the errors at the respective call site and just return instead of jumping around. Thanks, Lorenzo
Hello Lorenzo! On Tue, 27 Feb 2018, Lorenzo Pieralisi wrote: > On Tue, Feb 27, 2018 at 11:02:30AM +0100, Rolf Evers-Fischer wrote: > > From: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> > > > > 'put_device()' calls the relase function 'pci_epf_dev_release()', > > which already frees 'epf->name' and 'epf'. > > > > Therefore we must not free them again after 'put_device()'. > > > > Fixes: 5e8cb4033807 ("PCI: endpoint: Add EP core layer to enable EP controller and EP functions") > > > > Signed-off-by: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com> > > --- > > drivers/pci/endpoint/pci-epf-core.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c > > index 1f2506f32bb9..1878a6776519 100644 > > --- a/drivers/pci/endpoint/pci-epf-core.c > > +++ b/drivers/pci/endpoint/pci-epf-core.c > > @@ -232,7 +232,7 @@ struct pci_epf *pci_epf_create(const char *name) > > > > put_dev: > > put_device(dev); > > - kfree(epf->name); > > + return ERR_PTR(ret); > > Another thing you could do, which would get rid of these multiple return > statements (yes there is another one) would consist in removing the goto > labels completely and handle the errors at the respective call site and > just return instead of jumping around. > Thank you for sharing this idea! I've already changed it locally and it seems that we can get rid of 11 code lines with that. It is being verified. If it really works, there will be a new commit soon. Kind regards, Rolf
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index 1f2506f32bb9..1878a6776519 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -232,7 +232,7 @@ struct pci_epf *pci_epf_create(const char *name) put_dev: put_device(dev); - kfree(epf->name); + return ERR_PTR(ret); free_epf: kfree(epf);