Skip to contentSkip to conversation

reset: hisilicon: fix potential NULL pointer dereference

PATCH Superseded Gustavo A. R. Silva 2 participants 3 messages Jul 19, 2018 – Jul 26, 2018 Common clk

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 reset: hisilicon: fix potential NULL pointer dereference

There is a potential execution path in which function platform_get_resource() returns NULL. If this happens, we will end up having a NULL pointer dereference.

Fix this by adding asanity check in order to avoid a NULL pointer dereference.

This code was detected with the help of Coccinelle.

Cc [email protected]
Fixes 97b7129cd2af ("reset: hisilicon: change the definition of hisi_reset_init")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/clk/hisilicon/reset.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/clk/hisilicon/reset.c +3 -0
index 2a5015c..5dfb48b 100644
@@ -109,6 +109,9 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
 		return NULL;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return NULL;
+
 	rstc->membase = devm_ioremap(&pdev->dev,
 				res->start, resource_size(res));
 	if (!rstc->membase)
Signature · 1 line
2.7.4
maintainer
#1 lore raw

Quoting Gustavo A. R. Silva (2018-07-18 18:58:45)

Quoted 9 lines

There is a potential execution path in which function platform_get_resource() returns NULL. If this happens, we will end up having a NULL pointer dereference. =

Fix this by adding asanity check in order to avoid a NULL pointer dereference.
=

Quoted 5 lines

This code was detected with the help of Coccinelle. =

Cc [email protected]
Fixes 97b7129cd2af ("reset: hisilicon: change the definition of hisi_res=

et_init")

drivers/clk/hisilicon/reset.c struct hisi_reset_controller *hisi_reset_init(struct =

Quoted 11 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/clk/hisilicon/reset.c | 3 +++
 1 file changed, 3 insertions(+)

=

drivers/clk/hisilicon/reset.c +0 -0
index 2a5015c..5dfb48b 100644
@@ -109,6 +109,9 @@ struct hisi_reset_controller *hisi_reset_init(struct =

platform_device *pdev)

drivers/clk/hisilicon/reset.c struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev) · line 116

Quoted 9 lines
drivers/clk/hisilicon/reset.c +3 -0
                return NULL;
 =
 
        res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!res)
+               return NULL;
+
        rstc->membase =3D devm_ioremap(&pdev->dev,
                                res->start, resource_size(res));

Why can't we use devm_ioremap_resource() here instead?

author
#2 lore raw

Hi Stephen,

Quoted 24 lines · Stephen Boyd

Quoting Gustavo A. R. Silva (2018-07-18 18:58:45)

Quoted 23 lines

There is a potential execution path in which function platform_get_resource() returns NULL. If this happens, we will end up having a NULL pointer dereference.

Fix this by adding asanity check in order to avoid a NULL pointer dereference.

This code was detected with the help of Coccinelle.

Cc [email protected]
Fixes 97b7129cd2af ("reset: hisilicon: change the definition of hisi_reset_init")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/clk/hisilicon/reset.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/clk/hisilicon/reset.c +0 -0
index 2a5015c..5dfb48b 100644
@@ -109,6 +109,9 @@ struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev)
                return NULL;
Quoted 9 lines

drivers/clk/hisilicon/reset.c struct hisi_reset_controller *hisi_reset_init(struct platform_device *pdev) · line 116

Quoted 6 lines
drivers/clk/hisilicon/reset.c +3 -0
        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+       if (!res)
+               return NULL;
+
        rstc->membase = devm_ioremap(&pdev->dev,
                                res->start, resource_size(res));

Why can't we use devm_ioremap_resource() here instead?

You're right. I think we can perfectly use devm_ioremap_resource here and remove the null check.

I'll send patch for this shortly.

Thanks
--
Gustavo