There are several issues related to the error handling paths of
gp_aux_bus_probe():
- some resources may be released twice. Once explicitly in the error
handling path, and once via the release() function
- auxiliary_device_delete() should be called after the first successful
auxiliary_device_add()
To fix them, reorder the code:
- move the place where we get the irq for the 2nd wrapper.
- call kfree() and ida_free() after error checks, rather then in the
error handling path.
- have the error handling path look like the remove function
Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for the PIO function in the multi-function endpoint of pci1xxxx device.")
Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>
---
This patch is speculative and untested, review with care.
Other solutions are possible.
For example, we could use devm_kzalloc() to simplify the error handling
path and the release function.
---
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 74 ++++++++++---------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
index 32af2b14ff34..d3253e98f2ec 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
@@ -32,7 +32,7 @@ static void gp_auxiliary_device_release(struct device *dev)
static int gp_aux_bus_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct aux_bus_device *aux_bus;
- int retval;
+ int irq, retval;
retval = pcim_enable_device(pdev);
if (retval)
@@ -48,8 +48,10 @@ static int gp_aux_bus_probe(struct pci_dev *pdev, const struct pci_device_id *id
return -ENOMEM;
retval = ida_alloc(&gp_client_ida, GFP_KERNEL);
- if (retval < 0)
- goto err_ida_alloc_0;
+ if (retval < 0) {
+ kfree(aux_bus->aux_device_wrapper[0]);
+ return retval;
+ }
aux_bus->aux_device_wrapper[0]->aux_dev.name = aux_dev_otp_e2p_name;
aux_bus->aux_device_wrapper[0]->aux_dev.dev.parent = &pdev->dev;
@@ -60,21 +62,38 @@ static int gp_aux_bus_probe(struct pci_dev *pdev, const struct pci_device_id *id
aux_bus->aux_device_wrapper[0]->gp_aux_data.region_length = pci_resource_end(pdev, 0);
retval = auxiliary_device_init(&aux_bus->aux_device_wrapper[0]->aux_dev);
- if (retval < 0)
- goto err_aux_dev_init_0;
+ if (retval < 0) {
+ ida_free(&gp_client_ida, aux_bus->aux_device_wrapper[0]->aux_dev.id);
+ kfree(aux_bus->aux_device_wrapper[0]);
+ goto err_aux_dev_uninit_0;
+ }
retval = auxiliary_device_add(&aux_bus->aux_device_wrapper[0]->aux_dev);
if (retval)
- goto err_aux_dev_add_0;
+ goto err_aux_dev_uninit_0;
+
+
+ retval = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+ if (retval < 0)
+ goto err_aux_dev_del_0;
+
+ retval = pci_irq_vector(pdev, 0);
+ if (retval < 0)
+ goto err_aux_dev_del_0;
+ irq = retval;
aux_bus->aux_device_wrapper[1] = kzalloc(sizeof(*aux_bus->aux_device_wrapper[1]),
GFP_KERNEL);
- if (!aux_bus->aux_device_wrapper[1])
- return -ENOMEM;
+ if (!aux_bus->aux_device_wrapper[1]) {
+ retval = -ENOMEM;
+ goto err_aux_dev_del_0;
+ }
retval = ida_alloc(&gp_client_ida, GFP_KERNEL);
- if (retval < 0)
- goto err_ida_alloc_1;
+ if (retval < 0) {
+ kfree(aux_bus->aux_device_wrapper[1]);
+ goto err_aux_dev_del_0;
+ }
aux_bus->aux_device_wrapper[1]->aux_dev.name = aux_dev_gpio_name;
aux_bus->aux_device_wrapper[1]->aux_dev.dev.parent = &pdev->dev;
@@ -84,49 +103,34 @@ static int gp_aux_bus_probe(struct pci_dev *pdev, const struct pci_device_id *id
aux_bus->aux_device_wrapper[1]->gp_aux_data.region_start = pci_resource_start(pdev, 0);
aux_bus->aux_device_wrapper[1]->gp_aux_data.region_length = pci_resource_end(pdev, 0);
- retval = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
-
- if (retval < 0)
- goto err_aux_dev_init_1;
-
- retval = pci_irq_vector(pdev, 0);
- if (retval < 0)
- goto err_aux_dev_init_1;
-
pdev->irq = retval;
aux_bus->aux_device_wrapper[1]->gp_aux_data.irq_num = pdev->irq;
retval = auxiliary_device_init(&aux_bus->aux_device_wrapper[1]->aux_dev);
- if (retval < 0)
- goto err_aux_dev_init_1;
+ if (retval < 0) {
+ ida_free(&gp_client_ida, aux_bus->aux_device_wrapper[1]->aux_dev.id);
+ kfree(aux_bus->aux_device_wrapper[1]);
+ goto err_aux_dev_del_0;
+ }
retval = auxiliary_device_add(&aux_bus->aux_device_wrapper[1]->aux_dev);
if (retval)
- goto err_aux_dev_add_1;
+ goto err_aux_dev_uninit_1;
pci_set_drvdata(pdev, aux_bus);
pci_set_master(pdev);
return 0;
-err_aux_dev_add_1:
+err_aux_dev_uninit_1:
auxiliary_device_uninit(&aux_bus->aux_device_wrapper[1]->aux_dev);
-err_aux_dev_init_1:
- ida_free(&gp_client_ida, aux_bus->aux_device_wrapper[1]->aux_dev.id);
-
-err_ida_alloc_1:
- kfree(aux_bus->aux_device_wrapper[1]);
+err_aux_dev_del_0:
+ auxiliary_device_delete(&aux_bus->aux_device_wrapper[0]->aux_dev);
-err_aux_dev_add_0:
+err_aux_dev_uninit_0:
auxiliary_device_uninit(&aux_bus->aux_device_wrapper[0]->aux_dev);
-err_aux_dev_init_0:
- ida_free(&gp_client_ida, aux_bus->aux_device_wrapper[0]->aux_dev.id);
-
-err_ida_alloc_0:
- kfree(aux_bus->aux_device_wrapper[0]);
-
return retval;
}
--
2.34.1
