2019-04-03 16:04:46

by Wen Yang

[permalink] [raw]
Subject: [PATCH 0/7] fix possible object reference leaks in drivers/gpu

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

This patch series fix those possible object reference leaks in drivers/gpu.

Wen Yang (7):
drm/mediatek: fix possible object reference leak
drm/meson: fix possible object reference leak
drm/msm: a5xx: fix possible object reference leak
drm/omap: fix possible object reference leak
drm/pl111: fix possible object reference leak
drm: rcar-du: fix possible object reference leak
drm/tegra: fix possible object reference leak

drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 +
drivers/gpu/drm/meson/meson_dw_hdmi.c | 7 ++-----
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++++++----
drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 5 ++++-
drivers/gpu/drm/pl111/pl111_versatile.c | 4 ++++
drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 ++
drivers/gpu/drm/tegra/output.c | 2 ++
drivers/gpu/drm/tegra/rgb.c | 8 ++++++--
8 files changed, 27 insertions(+), 12 deletions(-)

--
2.9.5


2019-04-03 16:04:51

by Wen Yang

[permalink] [raw]
Subject: [PATCH 1/7] drm/mediatek: fix possible object reference leak

The call to of_parse_phandle 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/mediatek/mtk_hdmi.c:1521:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1509, but without a corresponding object release within this function.
drivers/gpu/drm/mediatek/mtk_hdmi.c:1524:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1509, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: CK Hu <[email protected]>
Cc: Philipp Zabel <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 915cc84..ed10f4d 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -1516,6 +1516,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
of_node_put(remote);

hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np);
+ of_node_put(i2c_np);
if (!hdmi->ddc_adpt) {
dev_err(dev, "Failed to get ddc i2c adapter by node\n");
return -EINVAL;
--
2.9.5

2019-04-03 16:05:02

by Wen Yang

[permalink] [raw]
Subject: [PATCH 5/7] drm/pl111: fix possible object reference leak

The call to of_find_matching_node_and_match 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/pl111/pl111_versatile.c:333:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.
drivers/gpu/drm/pl111/pl111_versatile.c:340:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.
drivers/gpu/drm/pl111/pl111_versatile.c:346:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.
drivers/gpu/drm/pl111/pl111_versatile.c:354:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.
drivers/gpu/drm/pl111/pl111_versatile.c:395:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.
drivers/gpu/drm/pl111/pl111_versatile.c:402:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 317, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Eric Anholt <[email protected]> (supporter:DRM DRIVER FOR ARM PL111 CLCD)
Cc: David Airlie <[email protected]> (maintainer:DRM DRIVERS)
Cc: Daniel Vetter <[email protected]> (maintainer:DRM DRIVERS)
Cc: [email protected] (open list:DRM DRIVERS)
Cc: [email protected] (open list)
---
drivers/gpu/drm/pl111/pl111_versatile.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c
index b9baefd..1c318ad 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -330,6 +330,7 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
ret = vexpress_muxfpga_init();
if (ret) {
dev_err(dev, "unable to initialize muxfpga driver\n");
+ of_node_put(np);
return ret;
}

@@ -337,17 +338,20 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
pdev = of_find_device_by_node(np);
if (!pdev) {
dev_err(dev, "can't find the sysreg device, deferring\n");
+ of_node_put(np);
return -EPROBE_DEFER;
}
map = dev_get_drvdata(&pdev->dev);
if (!map) {
dev_err(dev, "sysreg has not yet probed\n");
platform_device_put(pdev);
+ of_node_put(np);
return -EPROBE_DEFER;
}
} else {
map = syscon_node_to_regmap(np);
}
+ of_node_put(np);

if (IS_ERR(map)) {
dev_err(dev, "no Versatile syscon regmap\n");
--
2.9.5

2019-04-03 16:05:10

by Wen Yang

[permalink] [raw]
Subject: [PATCH 7/7] drm/tegra: fix possible object reference leak

The call to of_get_child_by_name 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/tegra/rgb.c:225:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 223, but without a corresponding object release within this function.
./drivers/gpu/drm/tegra/rgb.c:229:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 223, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Jonathan Hunter <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/gpu/drm/tegra/output.c | 2 ++
drivers/gpu/drm/tegra/rgb.c | 8 ++++++--
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 9c2b9da..78553d7 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -193,6 +193,8 @@ void tegra_output_remove(struct tegra_output *output)

if (output->ddc)
put_device(&output->ddc->dev);
+ if (output->of_node)
+ of_node_put(output->of_node);
}

int tegra_output_init(struct drm_device *drm, struct tegra_output *output)
diff --git a/drivers/gpu/drm/tegra/rgb.c b/drivers/gpu/drm/tegra/rgb.c
index 28a78d3..ad683a2 100644
--- a/drivers/gpu/drm/tegra/rgb.c
+++ b/drivers/gpu/drm/tegra/rgb.c
@@ -221,12 +221,16 @@ int tegra_dc_rgb_probe(struct tegra_dc *dc)
int err;

np = of_get_child_by_name(dc->dev->of_node, "rgb");
- if (!np || !of_device_is_available(np))
+ if (!np || !of_device_is_available(np)) {
+ of_node_put(np);
return -ENODEV;
+ }

rgb = devm_kzalloc(dc->dev, sizeof(*rgb), GFP_KERNEL);
- if (!rgb)
+ if (!rgb) {
+ of_node_put(np);
return -ENOMEM;
+ }

rgb->output.dev = dc->dev;
rgb->output.of_node = np;
--
2.9.5

2019-04-03 16:05:35

by Wen Yang

[permalink] [raw]
Subject: [PATCH 4/7] 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]>
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]
---
drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 5 ++++-
1 file changed, 4 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..60067e8 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
@@ -208,8 +208,10 @@ static int __init omapdss_boot_init(void)

dss = of_find_matching_node(NULL, omapdss_of_match);

- if (dss == NULL || !of_device_is_available(dss))
+ if (dss == NULL || !of_device_is_available(dss)) {
+ of_node_put(dss);
return 0;
+ }

omapdss_walk_device(dss, true);

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

+ of_node_put(dss);
return 0;
}

--
2.9.5

2019-04-03 16:06:07

by Wen Yang

[permalink] [raw]
Subject: [PATCH 3/7] drm/msm: a5xx: fix possible object reference leak

The call to of_get_child_by_name 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/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.

Signed-off-by: Wen Yang <[email protected]>
Cc: Rob Clark <[email protected]>
Cc: Sean Paul <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Jordan Crouse <[email protected]>
Cc: Mamta Shukla <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Sharat Masetty <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected] (open list)
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index d5f5e56..270da14 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -34,7 +34,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname)
{
struct device *dev = &gpu->pdev->dev;
const struct firmware *fw;
- struct device_node *np;
+ struct device_node *np, *mem_np;
struct resource r;
phys_addr_t mem_phys;
ssize_t mem_size;
@@ -48,11 +48,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname)
if (!np)
return -ENODEV;

- np = of_parse_phandle(np, "memory-region", 0);
- if (!np)
+ mem_np = of_parse_phandle(np, "memory-region", 0);
+ of_node_put(np);
+ if (!mem_np)
return -EINVAL;

- ret = of_address_to_resource(np, 0, &r);
+ ret = of_address_to_resource(mem_np, 0, &r);
+ of_node_put(mem_np);
if (ret)
return ret;

--
2.9.5

2019-04-03 16:06:08

by Wen Yang

[permalink] [raw]
Subject: [PATCH 2/7] 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] (open list)
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 563953e..109a933 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -720,13 +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);
+ if (remote)
+ return true;

return false;
}
--
2.9.5

2019-04-03 16:06:44

by Wen Yang

[permalink] [raw]
Subject: [PATCH 6/7] 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]>
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)
---
drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c
index afef696..30bceca 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
@@ -232,6 +232,8 @@ 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);
+ of_node_put(soc_node);
+ of_node_put(du_node);
return;
}

--
2.9.5

2019-04-04 13:28:02

by Laurent Pinchart

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

Hello Wen,

Thank you for the patch.

On Thu, Apr 04, 2019 at 12:04:12AM +0800, 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]>
> 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]

Reviewed-by: Laurent Pinchart <[email protected]>

Would you like to get the series merged in one go, or individual patches
picked by the respective maintainer ?

> ---
> drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c | 5 ++++-
> 1 file changed, 4 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..60067e8 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c
> @@ -208,8 +208,10 @@ static int __init omapdss_boot_init(void)
>
> dss = of_find_matching_node(NULL, omapdss_of_match);
>
> - if (dss == NULL || !of_device_is_available(dss))
> + if (dss == NULL || !of_device_is_available(dss)) {
> + of_node_put(dss);
> return 0;
> + }
>
> omapdss_walk_device(dss, true);
>
> @@ -234,6 +236,7 @@ static int __init omapdss_boot_init(void)
> kfree(n);
> }
>
> + of_node_put(dss);
> return 0;
> }
>

--
Regards,

Laurent Pinchart

2019-04-04 13:37:53

by Laurent Pinchart

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

Hi Wen,

Thank you for the patch.

On Thu, Apr 04, 2019 at 12:04:14AM +0800, 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]>
> 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)
> ---
> drivers/gpu/drm/rcar-du/rcar_du_of.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_of.c b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> index afef696..30bceca 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_of.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_of.c
> @@ -232,6 +232,8 @@ 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);
> + of_node_put(soc_node);
> + of_node_put(du_node);
> return;

Wouldn't it be simpler to just turn the return into a goto done ?

> }
>

--
Regards,

Laurent Pinchart

2019-04-04 20:00:10

by Eric Anholt

[permalink] [raw]
Subject: Re: [PATCH 5/7] drm/pl111: fix possible object reference leak

Wen Yang <[email protected]> writes:

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

Pushed to drm-misc-next. Thanks!


Attachments:
signature.asc (847.00 B)

2019-04-04 21:28:25

by Markus Elfring

[permalink] [raw]
Subject: Re: [PATCH 5/7] drm/pl111: fix possible object reference leak

> @@ -337,17 +338,20 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv)
> pdev = of_find_device_by_node(np);
> if (!pdev) {
> dev_err(dev, "can't find the sysreg device, deferring\n");
> + of_node_put(np);
> return -EPROBE_DEFER;
> }
> map = dev_get_drvdata(&pdev->dev);
> if (!map) {
> dev_err(dev, "sysreg has not yet probed\n");
> platform_device_put(pdev);
> + of_node_put(np);
> return -EPROBE_DEFER;
> }

How do you think about to move duplicate statements to an additional
jump target for the desired exception handling?

Regards,
Markus

2019-04-04 21:55:42

by Markus Elfring

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

> @@ -720,13 +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);

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/meson/meson_dw_hdmi.c?id=61de49cb596710b918f7a80839f0b6de2017bc32#n712

Can the order of these put calls matter (because of processor caches)?


> + if (remote)
> + return true;
>
> return false;

Would the use of a ternary operator be more succinct here?

+ return remote ? true : false;

Regards,
Markus

2019-04-04 21:57:22

by Markus Elfring

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

> @@ -208,8 +208,10 @@ static int __init omapdss_boot_init(void)
>
> dss = of_find_matching_node(NULL, omapdss_of_match);
>
> - if (dss == NULL || !of_device_is_available(dss))
> + if (dss == NULL || !of_device_is_available(dss)) {
> + of_node_put(dss);
> return 0;
> + }

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c?id=61de49cb596710b918f7a80839f0b6de2017bc32#n203

Can it be nicer to add a jump target here?

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


>
> omapdss_walk_device(dss, true);
>
> @@ -234,6 +236,7 @@ static int __init omapdss_boot_init(void)
> kfree(n);
> }
>

+put_node:

> + of_node_put(dss);
> return 0;
> }


Regards,
Markus

2019-04-09 05:52:50

by CK Hu (胡俊光)

[permalink] [raw]
Subject: Re: [PATCH 1/7] drm/mediatek: fix possible object reference leak

Hi, Wen:

On Thu, 2019-04-04 at 00:04 +0800, Wen Yang wrote:
> The call to of_parse_phandle 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/mediatek/mtk_hdmi.c:1521:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1509, but without a corresponding object release within this function.
> drivers/gpu/drm/mediatek/mtk_hdmi.c:1524:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 1509, but without a corresponding object release within this function.

For this patch, applied to mediatek-drm-fixes-5.1 [1], thanks.

[1]
https://github.com/ckhu-mediatek/linux.git-tags/commits/mediatek-drm-fixes-5.1

Regards,
CK

>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: CK Hu <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/gpu/drm/mediatek/mtk_hdmi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 915cc84..ed10f4d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -1516,6 +1516,7 @@ static int mtk_hdmi_dt_parse_pdata(struct mtk_hdmi *hdmi,
> of_node_put(remote);
>
> hdmi->ddc_adpt = of_find_i2c_adapter_by_node(i2c_np);
> + of_node_put(i2c_np);
> if (!hdmi->ddc_adpt) {
> dev_err(dev, "Failed to get ddc i2c adapter by node\n");
> return -EINVAL;


2019-04-10 18:59:28

by Jordan Crouse

[permalink] [raw]
Subject: Re: [PATCH 3/7] drm/msm: a5xx: fix possible object reference leak

On Thu, Apr 04, 2019 at 12:04:11AM +0800, Wen Yang wrote:
> The call to of_get_child_by_name 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/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 47, but without a corresponding object release within this function.
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:57:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:66:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:118:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 51, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <[email protected]>
> Cc: Rob Clark <[email protected]>
> Cc: Sean Paul <[email protected]>
> Cc: David Airlie <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Jordan Crouse <[email protected]>
> Cc: Mamta Shukla <[email protected]>
> Cc: Thomas Zimmermann <[email protected]>
> Cc: Sharat Masetty <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected] (open list)


Sorry for the delay. This looks right to me and possibly appropriate for stable
as well.

Reviewed-by: Jordan Crouse <[email protected]>

> ---
> drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index d5f5e56..270da14 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -34,7 +34,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname)
> {
> struct device *dev = &gpu->pdev->dev;
> const struct firmware *fw;
> - struct device_node *np;
> + struct device_node *np, *mem_np;
> struct resource r;
> phys_addr_t mem_phys;
> ssize_t mem_size;
> @@ -48,11 +48,13 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname)
> if (!np)
> return -ENODEV;
>
> - np = of_parse_phandle(np, "memory-region", 0);
> - if (!np)
> + mem_np = of_parse_phandle(np, "memory-region", 0);
> + of_node_put(np);
> + if (!mem_np)
> return -EINVAL;
>
> - ret = of_address_to_resource(np, 0, &r);
> + ret = of_address_to_resource(mem_np, 0, &r);
> + of_node_put(mem_np);
> if (ret)
> return ret;
>
> --
> 2.9.5
>

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project