| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230518163333.1355445-1-harshit.m.mogalapalli@oracle.com>
Date: Thu, 18 May 2023 09:33:33 -0700
From: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
To: unlisted-recipients:; (no To-header on input)
Cc: harshit.m.mogalapalli@...cle.com, error27@...il.com,
dan.carpenter@...aro.org, kernel-janitors@...r.kernel.org,
Kumaravel Thiagarajan <kumaravel.thiagarajan@...rochip.com>,
Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] misc: microchip: pci1xxxx: Fix error handling in gp_aux_bus_probe()
Smatch warns:
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c:73
gp_aux_bus_probe() warn: missing unwind goto?
Apart from above warning that smatch warns, we have other issues with
this function.
1. The call to auxiliary_device_add() needs a matching call to
auxiliary_device_delete(). When memory allocation for
"aux_bus->aux_device_wrapper[1]" fails we should also delete
auxiliary device for "aux_device_wrapper[0]".
2. In the error path when auxiliary_device_uninit() is called, it
does trigger the release function --> gp_auxiliary_device_release(),
this release function has the following:
ida_free(&gp_client_ida, aux_device_wrapper->aux_dev.id);
kfree(aux_device_wrapper);
so few error paths have double frees. Eg: The goto label
"err_aux_dev_add_0" first calls auxiliary_device_uninit() which also
does an ida_free(), so when the control reaches "err_aux_dev_init_0"
it will be a double free there.
Re-write the error handling code. Clean up manually before the
auxiliary_device_init() calls succeed and use gotos to clean up after
they succeed. Also change the goto label names to follow freeing the
last thing to make it more readable.
Fixes: 393fc2f5948f ("misc: microchip: pci1xxxx: load auxiliary bus driver for the PIO function in the multi-function endpoint of pci1xxxx device.")
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>
---
Only compile tested, from static analysis.
---
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c | 71 ++++++++++---------
1 file changed, 38 insertions(+), 33 deletions(-)
diff --git a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
index 32af2b14ff34..f76ef6fd7bfc 100644
--- a/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
+++ b/drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
@@ -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,28 @@ 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]);
+ return retval;
+ }
retval = auxiliary_device_add(&aux_bus->aux_device_wrapper[0]->aux_dev);
if (retval)
- goto err_aux_dev_add_0;
+ goto uninit_device_wrapper_0;
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 delete_device_wrapper_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 delete_device_wrapper_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;
@@ -85,48 +94,44 @@ 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_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;
+ 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 delete_device_wrapper_0;
+ }
retval = pci_irq_vector(pdev, 0);
- 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 delete_device_wrapper_0;
+ }
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 delete_device_wrapper_0;
+ }
retval = auxiliary_device_add(&aux_bus->aux_device_wrapper[1]->aux_dev);
if (retval)
- goto err_aux_dev_add_1;
+ goto uninit_device_wrapper_1;
pci_set_drvdata(pdev, aux_bus);
pci_set_master(pdev);
return 0;
-err_aux_dev_add_1:
+uninit_device_wrapper_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_add_0:
+delete_device_wrapper_0:
+ auxiliary_device_delete(&aux_bus->aux_device_wrapper[0]->aux_dev);
+uninit_device_wrapper_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.31.1
Powered by blists - more mailing lists