Skip to contentSkip to conversation

drm: sti: fix device leaks at component probe

PATCH Applied Johan Hovold 4 participants 10 messages Sep 22, 2025 – Nov 24, 2025

Subsystems (4)
Stable branch DRM drivers DRM misc DRM drivers for STI
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

PATCH 1/1 drm: sti: fix device leaks at component probe

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

Fixes cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
Cc [email protected] # 4.16
Cc Benjamin Gaignard <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/gpu/drm/sti/sti_vtg.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_vtg.c +6 -1
index ee81691b3203..ce6bc7e7b135 100644
@@ -143,12 +143,17 @@ struct sti_vtg {
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
 	struct platform_device *pdev;
+	struct sti_vtg *vtg;
 
 	pdev = of_find_device_by_node(np);
 	if (!pdev)
 		return NULL;
 
-	return (struct sti_vtg *)platform_get_drvdata(pdev);
+	vtg = platform_get_drvdata(pdev);
+
+	put_device(&pdev->dev);
+
+	return vtg;
 }
 
 static void vtg_reset(struct sti_vtg *vtg)
Signature · 1 line
2.49.1
maintainer
#2 lore raw
Quoted 3 lines · Markus Elfring
Quoted 1 line

Make sure to drop the references taken to the vtg devices by

VTG device?

Video Timing Generator. This IP creates a vsync pulse and synchonize the components together.

Quoted 8 lines
Quoted 4 lines

of_find_device_by_node() when looking up their driver data during component probe.

…

How do you think about to increase the application of scope-based resource management?
https://elixir.bootlin.com/linux/v6.17-rc7/source/include/linux/device.h#L1180

Oh... I wasn't aware of this. FWIU it is a way to directly free an allocated memory whenever a variable goes out of scope using the cleanup attribute.

IMO this is also a clever solution to prevent the memory leak, and it would be a shorter patch. So basically, instead of calling put_device() as Johan did, you would suggest something like this ?

drivers/gpu/drm/sti/sti_vtg.c +1 -1
index ee81691b3203..5193196d9291 100644
@@ -142,7 +142,7 @@ struct sti_vtg {
 
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
-       struct platform_device *pdev;
+       struct platform_device *pdev __free(put_device) = NULL;

Best regards,
Raphaël

Quoted 5 lines

Can a summary phrase like “Prevent device leak in of_vtg_find()” be nicer?

Regards,
Markus

#3 lore raw

…> +++ w/drivers/gpu/drm/sti/sti_vtg.c

drivers/gpu/drm/sti/sti_vtg.c struct sti_vtg { · line 145

Quoted 6 lines
drivers/gpu/drm/sti/sti_vtg.c +1 -1
@@ -142,7 +142,7 @@ struct sti_vtg {
 
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
-       struct platform_device *pdev;
+       struct platform_device *pdev __free(put_device) = NULL;

…

The scope may be reduced for such a variable.

Regards,
Markus

author
#4 lore raw
In reply to Raphaël Gallais-Pou

drivers/gpu/drm/sti/sti_vtg.c struct sti_vtg { · line 145

Quoted 10 lines · Raphaël Gallais-Pou
drivers/gpu/drm/sti/sti_vtg.c +1 -1
index ee81691b3203..5193196d9291 100644
@@ -142,7 +142,7 @@ struct sti_vtg {
 
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
-       struct platform_device *pdev;
+       struct platform_device *pdev __free(put_device) = NULL;

You'd need to declare the variable when looking up pdev, which is one of the reasons I don't like the cleanup helpers. It also often makes the code harder to reason about for no good reason (especially with some of the more esoteric cleanup helpers).

Keep it simple.

Johan

author
#5 lore raw
In reply to Johan Hovold
Quoted 4 lines · Johan Hovold

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Quoted 8 lines

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

Fixes cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
Cc [email protected] # 4.16
Cc Benjamin Gaignard <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Can this one be picked up for 6.19?

Johan

maintainer
#6 lore raw
In reply to Johan Hovold

Hi Johan,

For some reason this thread went through my filters, sorry.

Quoted 3 lines · Johan Hovold

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Markus suggested “Prevent device leak in of_vtg_find()” as commit summary.

Quoted 21 lines

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

Fixes cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
Cc [email protected] # 4.16
Cc Benjamin Gaignard <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/gpu/drm/sti/sti_vtg.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_vtg.c +0 -0
index ee81691b3203..ce6bc7e7b135 100644
@@ -143,12 +143,17 @@ struct sti_vtg {
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
 	struct platform_device *pdev;

drivers/gpu/drm/sti/sti_vtg.c struct sti_vtg { · line 154

Quoted 10 lines
drivers/gpu/drm/sti/sti_vtg.c +4 -1
+	struct sti_vtg *vtg;
 
 	pdev = of_find_device_by_node(np);
 	if (!pdev)
 		return NULL;
 
-	return (struct sti_vtg *)platform_get_drvdata(pdev);
+	vtg = platform_get_drvdata(pdev);
+
+	put_device(&pdev->dev);

I would prefer of_node_put() instead, which does the same basically, but at least it is more obviously linked to of_find_device_by_node().

Best regards,
Raphaël

Quoted 8 lines
drivers/gpu/drm/sti/sti_vtg.c +2 -0
+
+	return vtg;
 }
 
 static void vtg_reset(struct sti_vtg *vtg)
Signature · 1 line
2.49.1
author
#7 lore raw
Quoted 7 lines · Raphaël Gallais-Pou
Quoted 3 lines · Johan Hovold

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Markus suggested “Prevent device leak in of_vtg_find()” as commit summary.

Markus has gotten himself banned from the mailing lists some years ago and even if he is now back with a new mail address most of us still ignore him.

I prefer the Subject as it stands since it captures when the leaks happens, but I don't mind mentioning of_vtg_find() instead if you insist.

Quoted 26 lines
Quoted 26 lines

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

Fixes cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
Cc [email protected] # 4.16
Cc Benjamin Gaignard <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/gpu/drm/sti/sti_vtg.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/gpu/drm/sti/sti_vtg.c +1 -0
index ee81691b3203..ce6bc7e7b135 100644
@@ -143,12 +143,17 @@ struct sti_vtg {
 struct sti_vtg *of_vtg_find(struct device_node *np)
 {
 	struct platform_device *pdev;
+	struct sti_vtg *vtg;
 
 	pdev = of_find_device_by_node(np);
 	if (!pdev)
 		return NULL;
Quoted 7 lines

drivers/gpu/drm/sti/sti_vtg.c struct sti_vtg { · line 154

Quoted 4 lines
drivers/gpu/drm/sti/sti_vtg.c +3 -1
-	return (struct sti_vtg *)platform_get_drvdata(pdev);
+	vtg = platform_get_drvdata(pdev);
+
+	put_device(&pdev->dev);

I would prefer of_node_put() instead, which does the same basically, but at least it is more obviously linked to of_find_device_by_node().

of_node_put() operates on OF nodes, but here it is the platform device that is leaking.

Quoted 3 lines

drivers/gpu/drm/sti/sti_vtg.c struct sti_vtg { · line 157

Quoted 3 lines
drivers/gpu/drm/sti/sti_vtg.c +2 -0
+
+	return vtg;
 }

Johan

author
#8 lore raw
Quoted 10 lines · Johan Hovold
Quoted 7 lines · Raphaël Gallais-Pou
Quoted 3 lines · Johan Hovold

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Markus suggested “Prevent device leak in of_vtg_find()” as commit summary.

Quoted 7 lines

Markus has gotten himself banned from the mailing lists some years ago and even if he is now back with a new mail address most of us still ignore him.

I prefer the Subject as it stands since it captures when the leaks happens, but I don't mind mentioning of_vtg_find() instead if you insist.

Can this one be picked up for 6.19 or do you want me to respin?

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

Fixes cc6b741c6f63 ("drm: sti: remove useless fields from vtg structure")
Cc [email protected] # 4.16
Cc Benjamin Gaignard <[email protected]>
Signed-off-by Johan Hovold <[email protected]>

Johan

Applied
#9 lore raw
In reply to Johan Hovold
Quoted 9 lines · Johan Hovold

Make sure to drop the references taken to the vtg devices by of_find_device_by_node() when looking up their driver data during component probe.

Note that holding a reference to a platform device does not prevent its driver data from going away so there is no point in keeping the reference after the lookup helper returns.

[...]

Hi Johan,

Applied, thanks!

[1/1] drm: sti: fix device leaks at component probe

      commit: 620a8f131154250f6a64a07d049a4f235d6451a5

Best regards,

Signature · 1 line
Raphael Gallais-Pou <[email protected]>