2019-04-08 02:59:21

by Wen Yang

[permalink] [raw]
Subject: [PATCH v2] drm/meson: fix possible object reference leak

The call to of_graph_get_remote_port returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
drivers/gpu/drm/meson/meson_dw_hdmi.c:725:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 722, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Kevin Hilman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: Markus Elfring <[email protected]>
---
v2->v1: convert a if statement into a ternary statement.

drivers/gpu/drm/meson/meson_dw_hdmi.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 563953e..826b98b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -720,15 +720,10 @@ static bool meson_hdmi_connector_is_available(struct device *dev)

/* If the endpoint node exists, consider it enabled */
remote = of_graph_get_remote_port(ep);
- if (remote) {
- of_node_put(ep);
- return true;
- }
-
of_node_put(ep);
of_node_put(remote);

- return false;
+ return remote ? true : false;
}

static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
--
2.9.5


2019-04-08 02:58:41

by Wen Yang

[permalink] [raw]
Subject: [PATCH v2] drm: rcar-du: fix possible object reference leak

The call to of_get_parent returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
drivers/gpu/drm/rcar-du/rcar_du_of.c:235:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 216, but without a corresponding object release within this function.
drivers/gpu/drm/rcar-du/rcar_du_of.c:236:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Suggested-by: Laurent Pinchart <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Kieran Bingham <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected] (open list)
---
v2->v1: turn the return into a goto done.

drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c
index afef696..782bfc7 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -232,7 +232,7 @@ static void __init rcar_du_of_lvds_patch(const struct of_device_id *of_ids)
lvds_node = of_find_compatible_node(NULL, NULL, compatible);
if (lvds_node) {
of_node_put(lvds_node);
- return;
+ goto done;
}

/*
--
2.9.5

2019-04-08 02:58:41

by Wen Yang

[permalink] [raw]
Subject: [PATCH v2] drm/omap: fix possible object reference leak

The call to of_find_matching_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.

Detected by coccinelle with the following warnings:
drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.
drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
Cc: Tomi Valkeinen <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Sebastian Reichel <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Markus Elfring <[email protected]>
---
v2->v1: add a jump target.

drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
index 2b41c75..13b3b4a 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
@@ -209,7 +209,7 @@ static int __init omapdss_boot_init(void)
dss = of_find_matching_node(NULL, omapdss_of_match);

if (dss == NULL || !of_device_is_available(dss))
- return 0;
+ goto put_node;

omapdss_walk_device(dss, true);

@@ -234,6 +234,8 @@ static int __init omapdss_boot_init(void)
kfree(n);
}

+put_node:
+ of_node_put(dss);
return 0;
}

--
2.9.5

2019-04-08 08:47:08

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH v2] drm: rcar-du: fix possible object reference leak


On 4/8/2019 8:28 AM, Wen Yang wrote:
> The call to of_get_parent returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> drivers/gpu/drm/rcar-du/rcar_du_of.c:235:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 216, but without a corresponding object release within this function.
> drivers/gpu/drm/rcar-du/rcar_du_of.c:236:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <[email protected]>
> Suggested-by: Laurent Pinchart <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: Kieran Bingham <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected] (open list)
> ---
> v2->v1: turn the return into a goto done.
>
> drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> index afef696..782bfc7 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -232,7 +232,7 @@ static void __init rcar_du_of_lvds_patch(const struct of_device_id *of_ids)
> lvds_node = of_find_compatible_node(NULL, NULL, compatible);
> if (lvds_node) {
> of_node_put(lvds_node);
> - return;
> + goto done;
> }
>


you might have to create multiple level to do handling it.. there are
unnecessary put being done on "done" which is not valid

for this case.

-Mukesh

> /*

2019-04-08 08:50:53

by Mukesh Ojha

[permalink] [raw]
Subject: Re: [PATCH v2] drm/omap: fix possible object reference leak


On 4/8/2019 8:28 AM, Wen Yang wrote:
> The call to of_find_matching_node returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:212:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.
> drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c:237:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 209, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <[email protected]>
Reviewed-by: Mukesh Ojha <[email protected]>

Cheers,
-Mukesh

> Reviewed-by: Laurent Pinchart <[email protected]>
> Cc: Tomi Valkeinen <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Sebastian Reichel <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Markus Elfring <[email protected]>
> ---
> v2->v1: add a jump target.
>
> drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> index 2b41c75..13b3b4a 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> @@ -209,7 +209,7 @@ static int __init omapdss_boot_init(void)
> dss = of_find_matching_node(NULL, omapdss_of_match);
>
> if (dss == NULL || !of_device_is_available(dss))
> - return 0;
> + goto put_node;
>
> omapdss_walk_device(dss, true);
>
> @@ -234,6 +234,8 @@ static int __init omapdss_boot_init(void)
> kfree(n);
> }
>
> +put_node:
> + of_node_put(dss);
> return 0;
> }
>

2019-04-08 09:15:29

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH v2] drm/meson: fix possible object reference leak

> v2->v1: convert a if statement into a ternary statement.

Would you like to omit the arrow in such version information?


> @@ -720,15 +720,10 @@ static bool meson_hdmi_connector_is_available(struct device *dev)
>
> /* If the endpoint node exists, consider it enabled */
> remote = of_graph_get_remote_port(ep);
> - if (remote) {
> - of_node_put(ep);
> - return true;
> - }
> -
> of_node_put(ep);
> of_node_put(remote);

Can a reordering of the passed variables be useful for such function calls?

+ of_node_put(remote);
+ of_node_put(ep);

Regards,
Markus

2019-04-08 16:40:47

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH v2] drm: rcar-du: fix possible object reference leak

> v2->v1: turn the return into a goto done.

* The version identification can be shorter, can't it?

* The expection handling should be completed for the implementation
of the function “rcar_du_of_lvds_patch” in a different way.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/rcar-du/rcar_du_of.c?id=ac5b84a1ffe93c9fb882c0f2bdfac1c33077b920#n195

How do you think about to add a few jump targets (at the end)?

Regards,
Markus