Skip to content

Commit

Permalink
misc: microchip: pci1xxxx: Fix the error handling paths of gp_aux_bus…
Browse files Browse the repository at this point in the history
…_probe()

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: 393fc2f ("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@wanadoo.fr>
  • Loading branch information
tititiou36 authored and intel-lab-lkp committed Sep 18, 2022
1 parent 43b0686 commit 089c163
Showing 1 changed file with 39 additions and 35 deletions.
74 changes: 39 additions & 35 deletions drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
}

Expand Down

0 comments on commit 089c163

Please sign in to comment.