* [PATCH v5 1/3] PCI: endpoint: Simplify name allocation for EPF device
2018-02-28 17:32 [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Rolf Evers-Fischer
@ 2018-02-28 17:32 ` Rolf Evers-Fischer
2018-02-28 17:32 ` [PATCH v5 2/3] PCI: endpoint: Fix kernel panic after put_device() Rolf Evers-Fischer
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Rolf Evers-Fischer @ 2018-02-28 17:32 UTC (permalink / raw)
To: kishon
Cc: lorenzo.pieralisi, bhelgaas, linux-pci, linux-kernel,
andy.shevchenko, Rolf Evers-Fischer
From: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com>
This commit replaces allocating and freeing the intermediate
'buf'/'func_name' with a combination of 'kstrndup()' and 'len'.
'len' is the required length of 'epf->name'.
'epf->name' should be either the first part of 'name' preceding the '.'
or the complete 'name', if there is no '.' in the name.
Signed-off-by: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com>
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
drivers/pci/endpoint/pci-epf-core.c | 22 ++++------------------
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 766ce1dca2ec..1f2506f32bb9 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -200,8 +200,7 @@ struct pci_epf *pci_epf_create(const char *name)
int ret;
struct pci_epf *epf;
struct device *dev;
- char *func_name;
- char *buf;
+ int len;
epf = kzalloc(sizeof(*epf), GFP_KERNEL);
if (!epf) {
@@ -209,20 +208,11 @@ struct pci_epf *pci_epf_create(const char *name)
goto err_ret;
}
- buf = kstrdup(name, GFP_KERNEL);
- if (!buf) {
- ret = -ENOMEM;
- goto free_epf;
- }
-
- func_name = buf;
- buf = strchrnul(buf, '.');
- *buf = '\0';
-
- epf->name = kstrdup(func_name, GFP_KERNEL);
+ len = strchrnul(name, '.') - name;
+ epf->name = kstrndup(name, len, GFP_KERNEL);
if (!epf->name) {
ret = -ENOMEM;
- goto free_func_name;
+ goto free_epf;
}
dev = &epf->dev;
@@ -238,16 +228,12 @@ struct pci_epf *pci_epf_create(const char *name)
if (ret)
goto put_dev;
- kfree(func_name);
return epf;
put_dev:
put_device(dev);
kfree(epf->name);
-free_func_name:
- kfree(func_name);
-
free_epf:
kfree(epf);
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/3] PCI: endpoint: Fix kernel panic after put_device()
2018-02-28 17:32 [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Rolf Evers-Fischer
2018-02-28 17:32 ` [PATCH v5 1/3] PCI: endpoint: Simplify name allocation for EPF device Rolf Evers-Fischer
@ 2018-02-28 17:32 ` Rolf Evers-Fischer
2018-02-28 17:32 ` [PATCH v5 3/3] PCI: endpoint: pci_epf_create: remove goto labels Rolf Evers-Fischer
2018-03-01 12:22 ` [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Lorenzo Pieralisi
3 siblings, 0 replies; 5+ messages in thread
From: Rolf Evers-Fischer @ 2018-02-28 17:32 UTC (permalink / raw)
To: kishon
Cc: lorenzo.pieralisi, bhelgaas, linux-pci, linux-kernel,
andy.shevchenko, Rolf Evers-Fischer
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>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.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);
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 3/3] PCI: endpoint: pci_epf_create: remove goto labels
2018-02-28 17:32 [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Rolf Evers-Fischer
2018-02-28 17:32 ` [PATCH v5 1/3] PCI: endpoint: Simplify name allocation for EPF device Rolf Evers-Fischer
2018-02-28 17:32 ` [PATCH v5 2/3] PCI: endpoint: Fix kernel panic after put_device() Rolf Evers-Fischer
@ 2018-02-28 17:32 ` Rolf Evers-Fischer
2018-03-01 12:22 ` [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Lorenzo Pieralisi
3 siblings, 0 replies; 5+ messages in thread
From: Rolf Evers-Fischer @ 2018-02-28 17:32 UTC (permalink / raw)
To: kishon
Cc: lorenzo.pieralisi, bhelgaas, linux-pci, linux-kernel,
andy.shevchenko, Rolf Evers-Fischer
From: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com>
Removes the goto labels completely, handles the errors at the
respective call site and just returns instead of jumping around.
Signed-off-by: Rolf Evers-Fischer <rolf.evers.fischer@aptiv.com>
---
drivers/pci/endpoint/pci-epf-core.c | 32 ++++++++++++--------------------
1 file changed, 12 insertions(+), 20 deletions(-)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 1878a6776519..59ed29e550e9 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -203,16 +203,14 @@ struct pci_epf *pci_epf_create(const char *name)
int len;
epf = kzalloc(sizeof(*epf), GFP_KERNEL);
- if (!epf) {
- ret = -ENOMEM;
- goto err_ret;
- }
+ if (!epf)
+ return ERR_PTR(-ENOMEM);
len = strchrnul(name, '.') - name;
epf->name = kstrndup(name, len, GFP_KERNEL);
if (!epf->name) {
- ret = -ENOMEM;
- goto free_epf;
+ kfree(epf);
+ return ERR_PTR(-ENOMEM);
}
dev = &epf->dev;
@@ -221,24 +219,18 @@ struct pci_epf *pci_epf_create(const char *name)
dev->type = &pci_epf_type;
ret = dev_set_name(dev, "%s", name);
- if (ret)
- goto put_dev;
+ if (ret) {
+ put_device(dev);
+ return ERR_PTR(ret);
+ }
ret = device_add(dev);
- if (ret)
- goto put_dev;
+ if (ret) {
+ put_device(dev);
+ return ERR_PTR(ret);
+ }
return epf;
-
-put_dev:
- put_device(dev);
- return ERR_PTR(ret);
-
-free_epf:
- kfree(epf);
-
-err_ret:
- return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(pci_epf_create);
--
2.16.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create()
2018-02-28 17:32 [PATCH v5 0/3] pci: endpoint: Fix double free in pci_epf_create() Rolf Evers-Fischer
` (2 preceding siblings ...)
2018-02-28 17:32 ` [PATCH v5 3/3] PCI: endpoint: pci_epf_create: remove goto labels Rolf Evers-Fischer
@ 2018-03-01 12:22 ` Lorenzo Pieralisi
3 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Pieralisi @ 2018-03-01 12:22 UTC (permalink / raw)
To: Rolf Evers-Fischer
Cc: kishon, bhelgaas, linux-pci, linux-kernel, andy.shevchenko
On Wed, Feb 28, 2018 at 06:32:17PM +0100, Rolf Evers-Fischer wrote:
> This is version 5 of a patchset to avoid double free in function
> 'pci_epf_create()'.
>
> When I accidentally created a new endpoint device with an empty name,
> the kernel warned about "attempted to be registered with empty name!"
> and crashed afterwards.
>
> It turned out that the crash was not caused by the 'device_add()'
> function itself, but by a double kfree of 'epf->name' and 'epf'.
>
> The first patch just simplifies the code, while the second patch
> fixes the problem. The third patch removes the goto labels.
>
> Thank you Andy and Kishon for your Ack/Review on v3 for patches 1 and 2.
> In v4 of these patches only the first lines of the commit messages
> have been changed. In v5 these two patches have not been changed.
> Therefore the 'Acked-By'/'Reviewed-By' lines have been added. I hope
> that's acceptable.
>
> Changes in v5:
> - Beautified the ugly part of Patch #3 (v4), where the correct return
> value was hidden under two levels of 'if'.
>
> Changes in v4:
> - s/pci/PCI and s/epf/EPF in the first line of
> recent commit messages (thanks, Bjorn!)
> - The new patch #3 removes the goto labels
> in function 'pci_epf_create()' (thanks, Lorenzo!)
>
> Changes in v3:
> - Matched to other pending pci endpoint commits (thanks, Bjorn!)
> - Added "Fixes" tag in patch 2 (thanks, Andy!)
>
> Changes in v2:
> - Based on feedback from Lorenzo, Andy and Kishon (thanks!)
> - Change IDs removed
> - First patch completely reworked in order to eliminate the
> need for the second 'kstrdup' allocation and the 'kfree' of
> the first allocation.
> It was tested with name="pci_epf_test.0" and name="pci_epb":
> The 'epf->name' was "pci_epf_test" or "pci_epb" (=unchanged).
>
> Rolf Evers-Fischer (3):
> PCI: endpoint: Simplify name allocation for EPF device
> PCI: endpoint: Fix kernel panic after put_device()
> PCI: endpoint: pci_epf_create: remove goto labels
>
> drivers/pci/endpoint/pci-epf-core.c | 52 +++++++++++--------------------------
> 1 file changed, 15 insertions(+), 37 deletions(-)
Applied to pci/endpoint for v4.17, thanks.
Lorenzo
^ permalink raw reply [flat|nested] 5+ messages in thread