2023-05-05 08:58:48

by Kefeng Wang

Subject: [PATCH] irqchip/mbigen: unify the error handling in mbigen_of_create_domain()

Dan Carpenter reported that commit fea087fc291b "irqchip/mbigen: move
to use bus_get_dev_root()" leads to the following Smatch static checker
warning:

drivers/irqchip/irq-mbigen.c:258 mbigen_of_create_domain()
error: potentially dereferencing uninitialized 'child'.

It should not cause a problem on real hardware, but better to fix the
warning, let's move the bus_get_dev_root() out of the loop, and unify
the error handling to silence it.

Cc: Dan Carpenter <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
---
drivers/irqchip/irq-mbigen.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index eada5e0e3eb9..5101a3fb11df 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -240,26 +240,27 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
struct irq_domain *domain;
struct device_node *np;
u32 num_pins;
+ int ret = 0;
+
+ parent = bus_get_dev_root(&platform_bus_type);
+ if (!parent)
+ return -ENODEV;

for_each_child_of_node(pdev->dev.of_node, np) {
if (!of_property_read_bool(np, "interrupt-controller"))
continue;

- parent = bus_get_dev_root(&platform_bus_type);
- if (parent) {
- child = of_platform_device_create(np, NULL, parent);
- put_device(parent);
- if (!child) {
- of_node_put(np);
- return -ENOMEM;
- }
+ child = of_platform_device_create(np, NULL, parent);
+ if (!child) {
+ ret = -ENOMEM;
+ break;
}

if (of_property_read_u32(child->dev.of_node, "num-pins",
&num_pins) < 0) {
dev_err(&pdev->dev, "No num-pins property\n");
- of_node_put(np);
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}

domain = platform_msi_create_device_domain(&child->dev, num_pins,
@@ -267,12 +268,16 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
&mbigen_domain_ops,
mgn_chip);
if (!domain) {
- of_node_put(np);
- return -ENOMEM;
+ ret = -ENOMEM;
+ break;
}
}

- return 0;
+ put_device(parent);
+ if (ret)
+ of_node_put(np);
+
+ return ret;
}

#ifdef CONFIG_ACPI
--
2.35.3

2023-05-16 10:45:11

by tip-bot2 for Alexey Makhalov

Subject: [irqchip: irq/irqchip-fixes] irqchip/mbigen: Unify the error handling in mbigen_of_create_domain()

The following commit has been merged into the irq/irqchip-fixes branch of irqchip:

Commit-ID: cddb536a73ef2c82ce04059dc61c65e85a355561
Gitweb: https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/cddb536a73ef2c82ce04059dc61c65e85a355561
Author: Kefeng Wang <[email protected]>
AuthorDate: Fri, 05 May 2023 17:06:54 +08:00
Committer: Marc Zyngier <[email protected]>
CommitterDate: Tue, 16 May 2023 11:19:29 +01:00

irqchip/mbigen: Unify the error handling in mbigen_of_create_domain()

Dan Carpenter reported that commit fea087fc291b "irqchip/mbigen: move
to use bus_get_dev_root()" leads to the following Smatch static checker
warning:

drivers/irqchip/irq-mbigen.c:258 mbigen_of_create_domain()
error: potentially dereferencing uninitialized 'child'.

It should not cause a problem on real hardware, but better to fix the
warning, let's move the bus_get_dev_root() out of the loop, and unify
the error handling to silence it.

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Kefeng Wang <[email protected]>
Signed-off-by: Marc Zyngier <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
drivers/irqchip/irq-mbigen.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/irqchip/irq-mbigen.c b/drivers/irqchip/irq-mbigen.c
index eada5e0..5101a3f 100644
--- a/drivers/irqchip/irq-mbigen.c
+++ b/drivers/irqchip/irq-mbigen.c
@@ -240,26 +240,27 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
struct irq_domain *domain;
struct device_node *np;
u32 num_pins;
+ int ret = 0;
+
+ parent = bus_get_dev_root(&platform_bus_type);
+ if (!parent)
+ return -ENODEV;

for_each_child_of_node(pdev->dev.of_node, np) {
if (!of_property_read_bool(np, "interrupt-controller"))
continue;

- parent = bus_get_dev_root(&platform_bus_type);
- if (parent) {
- child = of_platform_device_create(np, NULL, parent);
- put_device(parent);
- if (!child) {
- of_node_put(np);
- return -ENOMEM;
- }
+ child = of_platform_device_create(np, NULL, parent);
+ if (!child) {
+ ret = -ENOMEM;
+ break;
}

if (of_property_read_u32(child->dev.of_node, "num-pins",
&num_pins) < 0) {
dev_err(&pdev->dev, "No num-pins property\n");
- of_node_put(np);
- return -EINVAL;
+ ret = -EINVAL;
+ break;
}

domain = platform_msi_create_device_domain(&child->dev, num_pins,
@@ -267,12 +268,16 @@ static int mbigen_of_create_domain(struct platform_device *pdev,
&mbigen_domain_ops,
mgn_chip);
if (!domain) {
- of_node_put(np);
- return -ENOMEM;
+ ret = -ENOMEM;
+ break;
}
}

- return 0;
+ put_device(parent);
+ if (ret)
+ of_node_put(np);
+
+ return ret;
}

#ifdef CONFIG_ACPI