[PATCH] PCI: pnv_php: Use common error handling code in pnv_php_alloc_slot()

[PATCH] PCI: pnv_php: Use common error handling code in pnv_php_alloc_slot()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 11 Jun 2026 11:16:49 +0200

Use an additional label so that a bit of exception handling can be better
reused at the end of an if branch.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pci/hotplug/pnv_php.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index ff92a5c301b8..e448f1802002 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -791,16 +791,15 @@ static struct pnv_php_slot *pnv_php_alloc_slot(struct device_node *dn)
 		return NULL;
 
 	php_slot->name = kstrdup(label, GFP_KERNEL);
-	if (!php_slot->name) {
-		kfree(php_slot);
-		return NULL;
-	}
+	if (!php_slot->name)
+		goto free_php_slot;
 
 	/* Allocate workqueue for this slot's interrupt handling */
 	php_slot->wq = alloc_workqueue("pciehp-%s", WQ_PERCPU, 0, php_slot->name);
 	if (!php_slot->wq) {
 		SLOT_WARN(php_slot, "Cannot alloc workqueue\n");
 		kfree(php_slot->name);
+free_php_slot:
 		kfree(php_slot);
 		return NULL;
 	}
-- 
2.54.0

Re: [PATCH] PCI: pnv_php: Use common error handling code in pnv_php_alloc_slot()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The patch introduces spaghetti code by placing an error cleanup label inside the body of an intermediate 'if' block.

Pre-existing issues:
- [High] Calling SLOT_WARN() when alloc_workqueue() fails causes a NULL pointer dereference because php_slot->bus is uninitialized.
--

commit fc5acb6d5ef23950f12a496bbb0ba6bddebe9f87
Author: Markus Elfring <elfring@users.sourceforge.net>

PCI: pnv_php: Use common error handling code in pnv_php_alloc_slot()

This commit introduces a new goto label to reuse error handling when allocating
a PCI hotplug slot in pnv_php.

> diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
> index ff92a5c301b81..e448f18020026 100644
> --- a/drivers/pci/hotplug/pnv_php.c
> +++ b/drivers/pci/hotplug/pnv_php.c
[ ... ]
> @@ -791,16 +791,15 @@ static struct pnv_php_slot *pnv_php_alloc_slot(struct device_node *dn)
[ ... ]
>  	/* Allocate workqueue for this slot's interrupt handling */
>  	php_slot->wq = alloc_workqueue("pciehp-%s", WQ_PERCPU, 0, php_slot->name);
>  	if (!php_slot->wq) {
>  		SLOT_WARN(php_slot, "Cannot alloc workqueue\n");

[Severity: High]
This is a pre-existing issue, but can this call to SLOT_WARN() cause a NULL
pointer dereference?

Looking at pnv_php_alloc_slot(), php_slot->pdev and php_slot->bus are not
initialized until much later in the function. Since the structure is
kzalloc'd, SLOT_WARN() will evaluate with a NULL php_slot->pdev and fall back
to dev_warn(&(sl)->bus->dev, ...).

Because php_slot->bus is also NULL here, this calculates an offset from a NULL
pointer and passes it to dev_warn(), which will crash when it attempts to read
the device name.

>  		kfree(php_slot->name);
> +free_php_slot:

[Severity: Low]
Does placing a cleanup label inside an intermediate if block obscure the
control flow? 

Standard kernel coding style typically places cleanup labels sequentially at
the end of the function rather than embedding them inside failure paths.

>  		kfree(php_slot);
>  		return NULL;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/5cd04727-44c5-4ace-af86-d3c227234d4b@web.de?part=1