Re: [PATCH 1/7] irqchip: Fix potential resource leaks
On 06/24/2020 05:15 PM, Krzysztof Kozlowski wrote: On Tue, 23 Jun 2020 at 10:51, Tiezhu Yang wrote: There exists some potential resource leaks in the error path, fix them. This should be split per driver and per bug (although mostly in driver it's just one bug). Otherwise it is difficult to review, backport and revert. Thanks for your suggestion, I have split it into a patch series [1], I will resend it some days later due to git send-email always failed. [1] https://lore.kernel.org/patchwork/cover/1263192/ Best regards, Krzysztof Signed-off-by: Tiezhu Yang --- drivers/irqchip/irq-ath79-misc.c | 3 +++ drivers/irqchip/irq-csky-apb-intc.c | 3 +++ drivers/irqchip/irq-csky-mpintc.c | 26 -- drivers/irqchip/irq-davinci-aintc.c | 17 + drivers/irqchip/irq-davinci-cp-intc.c | 17 ++--- drivers/irqchip/irq-digicolor.c | 4 drivers/irqchip/irq-dw-apb-ictl.c | 11 --- drivers/irqchip/irq-loongson-htvec.c | 5 - drivers/irqchip/irq-ls1x.c| 4 +++- drivers/irqchip/irq-mscc-ocelot.c | 6 -- drivers/irqchip/irq-nvic.c| 2 ++ drivers/irqchip/irq-omap-intc.c | 4 +++- drivers/irqchip/irq-riscv-intc.c | 1 + drivers/irqchip/irq-s3c24xx.c | 20 +++- drivers/irqchip/irq-xilinx-intc.c | 1 + 15 files changed, 98 insertions(+), 26 deletions(-)
Re: [PATCH 1/7] irqchip: Fix potential resource leaks
On Tue, 23 Jun 2020 at 10:51, Tiezhu Yang wrote: > > There exists some potential resource leaks in the error path, fix them. This should be split per driver and per bug (although mostly in driver it's just one bug). Otherwise it is difficult to review, backport and revert. Best regards, Krzysztof > Signed-off-by: Tiezhu Yang > --- > drivers/irqchip/irq-ath79-misc.c | 3 +++ > drivers/irqchip/irq-csky-apb-intc.c | 3 +++ > drivers/irqchip/irq-csky-mpintc.c | 26 -- > drivers/irqchip/irq-davinci-aintc.c | 17 + > drivers/irqchip/irq-davinci-cp-intc.c | 17 ++--- > drivers/irqchip/irq-digicolor.c | 4 > drivers/irqchip/irq-dw-apb-ictl.c | 11 --- > drivers/irqchip/irq-loongson-htvec.c | 5 - > drivers/irqchip/irq-ls1x.c| 4 +++- > drivers/irqchip/irq-mscc-ocelot.c | 6 -- > drivers/irqchip/irq-nvic.c| 2 ++ > drivers/irqchip/irq-omap-intc.c | 4 +++- > drivers/irqchip/irq-riscv-intc.c | 1 + > drivers/irqchip/irq-s3c24xx.c | 20 +++- > drivers/irqchip/irq-xilinx-intc.c | 1 + > 15 files changed, 98 insertions(+), 26 deletions(-)
Re: [PATCH 1/7] irqchip: Fix potential resource leaks
On 06/23/2020 11:55 PM, Markus Elfring wrote:
There exists some potential resource leaks in the error path, fix them.
Will the tag “Fixes” become relevant for the commit message?
Hi Markus,
Thanks for your reply and suggestion.
This patch contains many files in drivers/irqchip,
maybe I should split it into some small patches if use the tag "Fixes"?
…
+++ b/drivers/irqchip/irq-nvic.c
@@ -94,6 +94,7 @@ static int __init nvic_of_init(struct device_node *node,
if (!nvic_irq_domain) {
pr_warn("Failed to allocate irq domain\n");
+ iounmap(nvic_base);
return -ENOMEM;
}
@@ -103,6 +104,7 @@ static int __init nvic_of_init(struct device_node *node,
if (ret) {
pr_warn("Failed to allocate irq chips\n");
irq_domain_remove(nvic_irq_domain);
+ iounmap(nvic_base);
return ret;
}
Can it helpful to add jump targets so that a bit of exception handling
can be better reused at the end of this function?
OK, no problem, I will do it in the v2.
By the way, I have resent this patch series due to git send-email failed,
https://lore.kernel.org/patchwork/cover/1261517/
Thanks,
Tiezhu
Regards,
Markus
Re: [PATCH 1/7] irqchip: Fix potential resource leaks
> There exists some potential resource leaks in the error path, fix them.
Will the tag “Fixes” become relevant for the commit message?
…
> +++ b/drivers/irqchip/irq-nvic.c
> @@ -94,6 +94,7 @@ static int __init nvic_of_init(struct device_node *node,
>
> if (!nvic_irq_domain) {
> pr_warn("Failed to allocate irq domain\n");
> + iounmap(nvic_base);
> return -ENOMEM;
> }
>
> @@ -103,6 +104,7 @@ static int __init nvic_of_init(struct device_node *node,
> if (ret) {
> pr_warn("Failed to allocate irq chips\n");
> irq_domain_remove(nvic_irq_domain);
> + iounmap(nvic_base);
> return ret;
> }
Can it helpful to add jump targets so that a bit of exception handling
can be better reused at the end of this function?
Regards,
Markus
[PATCH 1/7] irqchip: Fix potential resource leaks
There exists some potential resource leaks in the error path, fix them.
Signed-off-by: Tiezhu Yang
---
drivers/irqchip/irq-ath79-misc.c | 3 +++
drivers/irqchip/irq-csky-apb-intc.c | 3 +++
drivers/irqchip/irq-csky-mpintc.c | 26 --
drivers/irqchip/irq-davinci-aintc.c | 17 +
drivers/irqchip/irq-davinci-cp-intc.c | 17 ++---
drivers/irqchip/irq-digicolor.c | 4
drivers/irqchip/irq-dw-apb-ictl.c | 11 ---
drivers/irqchip/irq-loongson-htvec.c | 5 -
drivers/irqchip/irq-ls1x.c| 4 +++-
drivers/irqchip/irq-mscc-ocelot.c | 6 --
drivers/irqchip/irq-nvic.c| 2 ++
drivers/irqchip/irq-omap-intc.c | 4 +++-
drivers/irqchip/irq-riscv-intc.c | 1 +
drivers/irqchip/irq-s3c24xx.c | 20 +++-
drivers/irqchip/irq-xilinx-intc.c | 1 +
15 files changed, 98 insertions(+), 26 deletions(-)
diff --git a/drivers/irqchip/irq-ath79-misc.c b/drivers/irqchip/irq-ath79-misc.c
index 3d641bb..4a86a9e 100644
--- a/drivers/irqchip/irq-ath79-misc.c
+++ b/drivers/irqchip/irq-ath79-misc.c
@@ -144,6 +144,7 @@ static int __init ath79_misc_intc_of_init(
base = of_iomap(node, 0);
if (!base) {
pr_err("Failed to get MISC IRQ registers\n");
+ irq_dispose_mapping(irq);
return -ENOMEM;
}
@@ -151,6 +152,8 @@ static int __init ath79_misc_intc_of_init(
_irq_domain_ops, base);
if (!domain) {
pr_err("Failed to add MISC irqdomain\n");
+ iounmap(base);
+ irq_dispose_mapping(irq);
return -EINVAL;
}
diff --git a/drivers/irqchip/irq-csky-apb-intc.c
b/drivers/irqchip/irq-csky-apb-intc.c
index 5a2ec43..aaa4ac7 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -118,6 +118,7 @@ ck_intc_init_comm(struct device_node *node, struct
device_node *parent)
_generic_chip_ops, NULL);
if (!root_domain) {
pr_err("C-SKY Intc irq_domain_add failed.\n");
+ iounmap(reg_base);
return -ENOMEM;
}
@@ -126,6 +127,8 @@ ck_intc_init_comm(struct device_node *node, struct
device_node *parent)
IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN, 0, 0);
if (ret) {
pr_err("C-SKY Intc irq_alloc_gc failed.\n");
+ irq_domain_remove(root_domain);
+ iounmap(reg_base);
return -ENOMEM;
}
diff --git a/drivers/irqchip/irq-csky-mpintc.c
b/drivers/irqchip/irq-csky-mpintc.c
index a1534ed..c195e24 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -247,8 +247,10 @@ csky_mpintc_init(struct device_node *node, struct
device_node *parent)
if (INTCG_base == NULL) {
INTCG_base = ioremap(mfcr("cr<31, 14>"),
INTCL_SIZE*nr_cpu_ids + INTCG_SIZE);
- if (INTCG_base == NULL)
- return -EIO;
+ if (INTCG_base == NULL) {
+ ret = -EIO;
+ goto err_free;
+ }
INTCL_base = INTCG_base + INTCG_SIZE;
@@ -257,8 +259,10 @@ csky_mpintc_init(struct device_node *node, struct
device_node *parent)
root_domain = irq_domain_add_linear(node, nr_irq, _irqdomain_ops,
NULL);
- if (!root_domain)
- return -ENXIO;
+ if (!root_domain) {
+ ret = -ENXIO;
+ goto err_iounmap;
+ }
/* for every cpu */
for_each_present_cpu(cpu) {
@@ -270,12 +274,22 @@ csky_mpintc_init(struct device_node *node, struct
device_node *parent)
#ifdef CONFIG_SMP
ipi_irq = irq_create_mapping(root_domain, IPI_IRQ);
- if (!ipi_irq)
- return -EIO;
+ if (!ipi_irq) {
+ ret = -EIO;
+ goto err_domain_remove;
+ }
set_send_ipi(_mpintc_send_ipi, ipi_irq);
#endif
return 0;
+
+err_domain_remove:
+ irq_domain_remove(root_domain);
+err_iounmap:
+ iounmap(INTCG_base);
+err_free:
+ kfree(__trigger);
+ return ret;
}
IRQCHIP_DECLARE(csky_mpintc, "csky,mpintc", csky_mpintc_init);
diff --git a/drivers/irqchip/irq-davinci-aintc.c
b/drivers/irqchip/irq-davinci-aintc.c
index 810ccc4..12db502 100644
--- a/drivers/irqchip/irq-davinci-aintc.c
+++ b/drivers/irqchip/irq-davinci-aintc.c
@@ -96,7 +96,7 @@ void __init davinci_aintc_init(const struct
davinci_aintc_config *config)
resource_size(>reg));
if (!davinci_aintc_base) {
pr_err("%s: unable to ioremap register range\n", __func__);
- return;
+ goto err_release;
}
/* Clear all
