* [PATCH] arm/mach-omap2/display: fix possible object reference leak @ 2019-02-13 15:32 Peng Hao 2019-02-19 17:05 ` Tony Lindgren 0 siblings, 1 reply; 5+ messages in thread From: Peng Hao @ 2019-02-13 15:32 UTC (permalink / raw) To: tony; +Cc: Peng Hao, linux-omap, linux-kernel, linux-arm-kernel of_find_device_by_node() takes a reference to the struct device when it finds a match via get_device.When returning error we should call put_device. Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> --- arch/arm/mach-omap2/display.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c index f86b72d..c6aa9ed 100644 --- a/arch/arm/mach-omap2/display.c +++ b/arch/arm/mach-omap2/display.c @@ -258,6 +258,7 @@ static int __init omapdss_init_of(void) r = of_platform_populate(node, NULL, NULL, &pdev->dev); if (r) { pr_err("Unable to populate DSS submodule devices\n"); + put_device(&pdev->dev); return r; } -- 1.8.3.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] arm/mach-omap2/display: fix possible object reference leak 2019-02-13 15:32 [PATCH] arm/mach-omap2/display: fix possible object reference leak Peng Hao @ 2019-02-19 17:05 ` Tony Lindgren 2019-02-19 17:30 ` Julia Lawall 2019-02-19 17:33 ` Julia Lawall 0 siblings, 2 replies; 5+ messages in thread From: Tony Lindgren @ 2019-02-19 17:05 UTC (permalink / raw) To: Peng Hao Cc: devicetree, Rob Herring, linux-kernel, Julia Lawall, Tomi Valkeinen, linux-omap, linux-arm-kernel Hi, Adding devicetree list, Julia, Rob and Tomi to Cc. * Peng Hao <peng.hao2@zte.com.cn> [190212 23:11]: > of_find_device_by_node() takes a reference to the struct device > when it finds a match via get_device.When returning error we should > call put_device. > > Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> > --- > arch/arm/mach-omap2/display.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c > index f86b72d..c6aa9ed 100644 > --- a/arch/arm/mach-omap2/display.c > +++ b/arch/arm/mach-omap2/display.c > @@ -258,6 +258,7 @@ static int __init omapdss_init_of(void) > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > if (r) { > pr_err("Unable to populate DSS submodule devices\n"); > + put_device(&pdev->dev); > return r; > } In general, if the device tree node is never used afterwards, should this be just: r = of_platform_populate(node, NULL, NULL, &pdev->dev); of_node_put(dev_node); if (r) { ... } If so, Julia might have a Coccinelle recpipe for it? Regards, Tony _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm/mach-omap2/display: fix possible object reference leak 2019-02-19 17:05 ` Tony Lindgren @ 2019-02-19 17:30 ` Julia Lawall 2019-02-19 17:33 ` Julia Lawall 1 sibling, 0 replies; 5+ messages in thread From: Julia Lawall @ 2019-02-19 17:30 UTC (permalink / raw) To: Tony Lindgren Cc: Rob Herring, Peng Hao, devicetree, linux-kernel, Tomi Valkeinen, linux-omap, wen.yang99, linux-arm-kernel On Tue, 19 Feb 2019, Tony Lindgren wrote: > Hi, > > Adding devicetree list, Julia, Rob and Tomi to Cc. > > * Peng Hao <peng.hao2@zte.com.cn> [190212 23:11]: > > of_find_device_by_node() takes a reference to the struct device > > when it finds a match via get_device.When returning error we should > > call put_device. > > > > Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> > > --- > > arch/arm/mach-omap2/display.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c > > index f86b72d..c6aa9ed 100644 > > --- a/arch/arm/mach-omap2/display.c > > +++ b/arch/arm/mach-omap2/display.c > > @@ -258,6 +258,7 @@ static int __init omapdss_init_of(void) > > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > > if (r) { > > pr_err("Unable to populate DSS submodule devices\n"); > > + put_device(&pdev->dev); > > return r; > > } > > In general, if the device tree node is never used afterwards, > should this be just: > > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > of_node_put(dev_node); Are these solving the same problems? The of_node_put looks clearly necessary, whether there is a success or failure. I'm not familiar with put_device. I see that it does a kobject_put, but I don't know what happens because of that. But it looks like an inconsistency that Peng's patch only considers the failure case, while your suggestion happens always. I guess Peng's patch is motivated by a Coccinelle script that Wen Yang (also from ZTE) has been working on. Perhaps there is a need to adjust what is suggested by that script. [Wen Yang added to CC] julia > if (r) { > ... > } > > If so, Julia might have a Coccinelle recpipe for it? > > Regards, > > Tony > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm/mach-omap2/display: fix possible object reference leak 2019-02-19 17:05 ` Tony Lindgren 2019-02-19 17:30 ` Julia Lawall @ 2019-02-19 17:33 ` Julia Lawall 2019-02-19 17:58 ` Tony Lindgren 1 sibling, 1 reply; 5+ messages in thread From: Julia Lawall @ 2019-02-19 17:33 UTC (permalink / raw) To: Tony Lindgren Cc: Rob Herring, Peng Hao, devicetree, linux-kernel, Tomi Valkeinen, linux-omap, wen.yang99, linux-arm-kernel On Tue, 19 Feb 2019, Tony Lindgren wrote: > Hi, > > Adding devicetree list, Julia, Rob and Tomi to Cc. > > * Peng Hao <peng.hao2@zte.com.cn> [190212 23:11]: > > of_find_device_by_node() takes a reference to the struct device > > when it finds a match via get_device.When returning error we should > > call put_device. > > > > Signed-off-by: Peng Hao <peng.hao2@zte.com.cn> > > --- > > arch/arm/mach-omap2/display.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c > > index f86b72d..c6aa9ed 100644 > > --- a/arch/arm/mach-omap2/display.c > > +++ b/arch/arm/mach-omap2/display.c > > @@ -258,6 +258,7 @@ static int __init omapdss_init_of(void) > > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > > if (r) { > > pr_err("Unable to populate DSS submodule devices\n"); > > + put_device(&pdev->dev); > > return r; > > } > > In general, if the device tree node is never used afterwards, > should this be just: > > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > of_node_put(dev_node); > if (r) { > ... > } > > If so, Julia might have a Coccinelle recpipe for it? Unfortunately this is not really an ideal case for Coccinelle, because node is the result of calling a local function and Coccinelle doesn't by default do any interprocedural analysis. It is possible to write a rule that explicitly looks for one function that returns a device node and then the pattern of its usage in the caller, though. julia _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] arm/mach-omap2/display: fix possible object reference leak 2019-02-19 17:33 ` Julia Lawall @ 2019-02-19 17:58 ` Tony Lindgren 0 siblings, 0 replies; 5+ messages in thread From: Tony Lindgren @ 2019-02-19 17:58 UTC (permalink / raw) To: Julia Lawall Cc: Rob Herring, Peng Hao, devicetree, linux-kernel, Tomi Valkeinen, linux-omap, wen.yang99, linux-arm-kernel * Julia Lawall <julia.lawall@lip6.fr> [190219 17:33]: > On Tue, 19 Feb 2019, Tony Lindgren wrote: > > In general, if the device tree node is never used afterwards, > > should this be just: > > > > r = of_platform_populate(node, NULL, NULL, &pdev->dev); > > of_node_put(dev_node); > > if (r) { > > ... > > } > > > > If so, Julia might have a Coccinelle recpipe for it? > > Unfortunately this is not really an ideal case for Coccinelle, because > node is the result of calling a local function and Coccinelle doesn't by > default do any interprocedural analysis. It is possible to write a rule > that explicitly looks for one function that returns a device node and then > the pattern of its usage in the caller, though. OK thanks for the information. Regards, Tony _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-19 17:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-13 15:32 [PATCH] arm/mach-omap2/display: fix possible object reference leak Peng Hao 2019-02-19 17:05 ` Tony Lindgren 2019-02-19 17:30 ` Julia Lawall 2019-02-19 17:33 ` Julia Lawall 2019-02-19 17:58 ` Tony Lindgren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).