[PATCH] bus: vexpress-config: fix device reference leak

[PATCH] bus: vexpress-config: fix device reference leak

[Johan Hovold]

Make sure to drop the reference to the parent device taken by
class_find_device() after populating the bus.

Fixes: 3b9334ac835b ("mfd: vexpress: Convert custom func API to regmap")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/bus/vexpress-config.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/vexpress-config.c b/drivers/bus/vexpress-config.c
index 9efdf1de4035..493e7b9fc813 100644
--- a/drivers/bus/vexpress-config.c
+++ b/drivers/bus/vexpress-config.c
@@ -171,6 +171,7 @@ static int vexpress_config_populate(struct device_node *node)
 {
 	struct device_node *bridge;
 	struct device *parent;
+	int ret;
 
 	bridge = of_parse_phandle(node, "arm,vexpress,config-bridge", 0);
 	if (!bridge)
@@ -182,7 +183,11 @@ static int vexpress_config_populate(struct device_node *node)
 	if (WARN_ON(!parent))
 		return -ENODEV;
 
-	return of_platform_populate(node, NULL, NULL, parent);
+	ret = of_platform_populate(node, NULL, NULL, parent);
+
+	put_device(parent);
+
+	return ret;
 }
 
 static int __init vexpress_config_init(void)
-- 
2.7.3

[PATCH] bus: vexpress-config: fix device reference leak

[Pawel Moll]

On Tue, 2016-11-01 at 11:43 +0100, Johan Hovold wrote:
> Make sure to drop the reference to the parent device taken by
> class_find_device() after populating the bus.
> 
> Fixes: 3b9334ac835b ("mfd: vexpress: Convert custom func API to
> regmap")
> Signed-off-by: Johan Hovold <johan@kernel.org>

You're right. May I ask how did you figure it out? The get_device()
happening in class_find_device() is a bit obscure, so have you simply
followed places where it's being used or used some static (?) analysis
tool? If the latter, I'd be very curios to hear what was it :-)

Acked-by: Pawel Moll <pawel.moll@arm.com>

Thanks!

Pawel

[PATCH] bus: vexpress-config: fix device reference leak

[Russell King - ARM Linux]

On Fri, Nov 04, 2016 at 11:42:26AM +0000, Pawel Moll wrote:
> On Tue, 2016-11-01 at 11:43 +0100, Johan Hovold wrote:
> > Make sure to drop the reference to the parent device taken by
> > class_find_device() after populating the bus.
> > 
> > Fixes: 3b9334ac835b ("mfd: vexpress: Convert custom func API to
> > regmap")
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> You're right. May I ask how did you figure it out? The get_device()
> happening in class_find_device() is a bit obscure,

It's not obscure at all - all the functions that find a device do so
under a lock to ensure that the device does not go away, and they
take a reference count on the device before returning the pointer for
exactly the same reason.

If they didn't do that, the find function could locate a struct device
while another thread is deleting the struct device, and it would then
return a stale pointer - and dereferencing that pointer would then be
a use-after-free bug.

So not obscure, but rather fundamentally necessary.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] bus: vexpress-config: fix device reference leak

[Johan Hovold]

On Fri, Nov 04, 2016 at 11:42:26AM +0000, Pawel Moll wrote:
> On Tue, 2016-11-01 at 11:43 +0100, Johan Hovold wrote:
> > Make sure to drop the reference to the parent device taken by
> > class_find_device() after populating the bus.
> > 
> > Fixes: 3b9334ac835b ("mfd: vexpress: Convert custom func API to
> > regmap")
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> You're right. May I ask how did you figure it out? The get_device()
> happening in class_find_device() is a bit obscure, so have you simply
> followed places where it's being used or used some static (?) analysis
> tool? If the latter, I'd be very curios to hear what was it :-)

I stumbled over one of these leaks and grepped and searched for more.
I've submitted a few patches this week fixing some of the more obvious
ones, but there are more lurking behind various subsystem wrappers.

Thanks,
Johan

[PATCH] bus: vexpress-config: fix device reference leak

[Sudeep Holla]

On 01/11/16 10:43, Johan Hovold wrote:
> Make sure to drop the reference to the parent device taken by
> class_find_device() after populating the bus.
>

Thanks for the fix. I somehow missed this patch, sorry for that.
Not sure if arm-soc guys take this as a fix for v4.9, and I have already
sent PR for v4.10. I will repost this along with the ack and check with
them.

-- 
Regards,
Sudeep