backlight: fix device-tree node lookups

A number of drivers have been using the wrong OF helper when doing child-node
lookups during probe. This meant that they were doing tree-wide searches rather
than matching on child nodes and that the parent node could end up being
prematurely freed.

Johan

v2
 - add patch 4/4 which fixes two node leaks in as3711_bl instead of marking
   the leaks using FIXMEs in patch 1/4


Johan Hovold (4):
  backlight: as3711_bl: fix device-tree node lookup
  backlight: max8925_bl: fix device-tree node lookup
  backlight: tps65217_bl: fix device-tree node lookup
  backlight: as3711_bl: fix device-tree node leaks

 drivers/video/backlight/as3711_bl.c   | 45 ++++++++++++++++++++++-------------
 drivers/video/backlight/max8925_bl.c  |  4 +++-
 drivers/video/backlight/tps65217_bl.c |  4 ++--
 include/linux/mfd/as3711.h            |  4 ++--
 4 files changed, 36 insertions(+), 21 deletions(-)

-- 
2.15.0

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: stable <redacted>     # 3.10
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/as3711_bl.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
index 734a9158946b..e55304d5cf07 100644
--- a/drivers/video/backlight/as3711_bl.c
+++ b/drivers/video/backlight/as3711_bl.c

@@ -262,10 +262,10 @@ static int as3711_bl_register(struct platform_device *pdev,
 static int as3711_backlight_parse_dt(struct device *dev)
 {
 	struct as3711_bl_pdata *pdata = dev_get_platdata(dev);
-	struct device_node *bl -		of_find_node_by_name(dev->parent->of_node, "backlight"), *fb;
+	struct device_node *bl, *fb;
 	int ret;
 
+	bl = of_get_child_by_name(dev->parent->of_node, "backlight");
 	if (!bl) {
 		dev_dbg(dev, "backlight node not found\n");
 		return -ENODEV;

@@ -279,7 +279,7 @@ static int as3711_backlight_parse_dt(struct device *dev)
 		if (pdata->su1_max_uA <= 0)
 			ret = -EINVAL;
 		if (ret < 0)
-			return ret;
+			goto err_put_bl;
 	}
 
 	fb = of_parse_phandle(bl, "su2-dev", 0);

@@ -292,7 +292,7 @@ static int as3711_backlight_parse_dt(struct device *dev)
 		if (pdata->su2_max_uA <= 0)
 			ret = -EINVAL;
 		if (ret < 0)
-			return ret;
+			goto err_put_bl;
 
 		if (of_find_property(bl, "su2-feedback-voltage", NULL)) {
 			pdata->su2_feedback = AS3711_SU2_VOLTAGE;

@@ -314,8 +314,10 @@ static int as3711_backlight_parse_dt(struct device *dev)
 			pdata->su2_feedback = AS3711_SU2_CURR_AUTO;
 			count++;
 		}
-		if (count != 1)
-			return -EINVAL;
+		if (count != 1) {
+			ret = -EINVAL;
+			goto err_put_bl;
+		}
 
 		count = 0;
 		if (of_find_property(bl, "su2-fbprot-lx-sd4", NULL)) {

@@ -334,8 +336,10 @@ static int as3711_backlight_parse_dt(struct device *dev)
 			pdata->su2_fbprot = AS3711_SU2_GPIO4;
 			count++;
 		}
-		if (count != 1)
-			return -EINVAL;
+		if (count != 1) {
+			ret = -EINVAL;
+			goto err_put_bl;
+		}
 
 		count = 0;
 		if (of_find_property(bl, "su2-auto-curr1", NULL)) {

@@ -355,11 +359,20 @@ static int as3711_backlight_parse_dt(struct device *dev)
 		 * At least one su2-auto-curr* must be specified iff
 		 * AS3711_SU2_CURR_AUTO is used
 		 */
-		if (!count ^ (pdata->su2_feedback != AS3711_SU2_CURR_AUTO))
-			return -EINVAL;
+		if (!count ^ (pdata->su2_feedback != AS3711_SU2_CURR_AUTO)) {
+			ret = -EINVAL;
+			goto err_put_bl;
+		}
 	}
 
+	of_node_put(bl);
+
 	return 0;
+
+err_put_bl:
+	of_node_put(bl);
+
+	return ret;
 }
 
 static int as3711_backlight_probe(struct platform_device *pdev)

-- 
2.15.0

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed,
while the child backlight node was leaked.

Fixes: 47ec340cb8e2 ("mfd: max8925: Support dt for backlight")
Cc: stable <redacted>     # 3.9
Cc: Qing Xu <redacted>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/max8925_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/max8925_bl.c b/drivers/video/backlight/max8925_bl.c
index 7b738d60ecc2..f3aa6088f1d9 100644
--- a/drivers/video/backlight/max8925_bl.c
+++ b/drivers/video/backlight/max8925_bl.c

@@ -116,7 +116,7 @@ static void max8925_backlight_dt_init(struct platform_device *pdev)
 	if (!pdata)
 		return;
 
-	np = of_find_node_by_name(nproot, "backlight");
+	np = of_get_child_by_name(nproot, "backlight");
 	if (!np) {
 		dev_err(&pdev->dev, "failed to find backlight node\n");
 		return;

@@ -125,6 +125,8 @@ static void max8925_backlight_dt_init(struct platform_device *pdev)
 	if (!of_property_read_u32(np, "maxim,max8925-dual-string", &val))
 		pdata->dual_string = val;
 
+	of_node_put(np);
+
 	pdev->dev.platform_data = pdata;
 }
 

-- 
2.15.0

Two framebuffer device-node names were looked up during probe, but were
only used as flags to indicate the presence of two framebuffer device.

Drop the unused framebuffer name along with a likewise unused device
pointer from the driver data, and update the platform data to pass in
booleans instead of the framebuffer strings. This allows us do drop the
node references acquired during probe, which would otherwise leak.

Note that there are no other in-kernel users of the modified
platform-data fields.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/as3711_bl.c | 12 ++++++------
 include/linux/mfd/as3711.h          |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/backlight/as3711_bl.c b/drivers/video/backlight/as3711_bl.c
index e55304d5cf07..ca544aa764b8 100644
--- a/drivers/video/backlight/as3711_bl.c
+++ b/drivers/video/backlight/as3711_bl.c

@@ -28,8 +28,6 @@ enum as3711_bl_type {
 
 struct as3711_bl_data {
 	bool powered;
-	const char *fb_name;
-	struct device *fb_dev;
 	enum as3711_bl_type type;
 	int brightness;
 	struct backlight_device *bl;

@@ -273,7 +271,9 @@ static int as3711_backlight_parse_dt(struct device *dev)
 
 	fb = of_parse_phandle(bl, "su1-dev", 0);
 	if (fb) {
-		pdata->su1_fb = fb->full_name;
+		of_node_put(fb);
+
+		pdata->su1_fb = true;
 
 		ret = of_property_read_u32(bl, "su1-max-uA", &pdata->su1_max_uA);
 		if (pdata->su1_max_uA <= 0)

@@ -286,7 +286,9 @@ static int as3711_backlight_parse_dt(struct device *dev)
 	if (fb) {
 		int count = 0;
 
-		pdata->su2_fb = fb->full_name;
+		of_node_put(fb);
+
+		pdata->su2_fb = true;
 
 		ret = of_property_read_u32(bl, "su2-max-uA", &pdata->su2_max_uA);
 		if (pdata->su2_max_uA <= 0)

@@ -425,7 +427,6 @@ static int as3711_backlight_probe(struct platform_device *pdev)
 
 	if (pdata->su1_fb) {
 		su = &supply->su1;
-		su->fb_name = pdata->su1_fb;
 		su->type = AS3711_BL_SU1;
 
 		max_brightness = min(pdata->su1_max_uA, 31);

@@ -436,7 +437,6 @@ static int as3711_backlight_probe(struct platform_device *pdev)
 
 	if (pdata->su2_fb) {
 		su = &supply->su2;
-		su->fb_name = pdata->su2_fb;
 		su->type = AS3711_BL_SU2;
 
 		switch (pdata->su2_fbprot) {

diff --git a/include/linux/mfd/as3711.h b/include/linux/mfd/as3711.h
index 34cc85864be5..ddd0b953323b 100644
--- a/include/linux/mfd/as3711.h
+++ b/include/linux/mfd/as3711.h

@@ -108,9 +108,9 @@ struct as3711_regulator_pdata {
 };
 
 struct as3711_bl_pdata {
-	const char *su1_fb;
+	bool su1_fb;
 	int su1_max_uA;
-	const char *su2_fb;
+	bool su2_fb;
 	int su2_max_uA;
 	enum as3711_su2_feedback su2_feedback;
 	enum as3711_su2_fbprot su2_fbprot;

-- 
2.15.0

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

This would only cause trouble if the child node is missing while there
is an unrelated node named "backlight" elsewhere in the tree.

Fixes: eebfdc17cc6c ("backlight: Add TPS65217 WLED driver")
Cc: stable <redacted>     # 3.7
Cc: Matthias Kaehlcke <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/tps65217_bl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/tps65217_bl.c b/drivers/video/backlight/tps65217_bl.c
index 380917c86276..762e3feed097 100644
--- a/drivers/video/backlight/tps65217_bl.c
+++ b/drivers/video/backlight/tps65217_bl.c

@@ -184,11 +184,11 @@ static struct tps65217_bl_pdata *
 tps65217_bl_parse_dt(struct platform_device *pdev)
 {
 	struct tps65217 *tps = dev_get_drvdata(pdev->dev.parent);
-	struct device_node *node = of_node_get(tps->dev->of_node);
+	struct device_node *node;
 	struct tps65217_bl_pdata *pdata, *err;
 	u32 val;
 
-	node = of_find_node_by_name(node, "backlight");
+	node = of_get_child_by_name(tps->dev->of_node, "backlight");
 	if (!node)
 		return ERR_PTR(-ENODEV);
 

-- 
2.15.0

On 20/11/17 10:45, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: stable <redacted>     # 3.10
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>

Acked-by: Daniel Thompson <redacted>

48 more quoted lines

On 20/11/17 10:45, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed,
while the child backlight node was leaked.

Fixes: 47ec340cb8e2 ("mfd: max8925: Support dt for backlight")
Cc: stable <redacted>     # 3.9
Cc: Qing Xu <redacted>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>

Didn't I already ack this one?

Acked-by: Daniel Thompson <redacted>

On 20/11/17 10:45, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

This would only cause trouble if the child node is missing while there
is an unrelated node named "backlight" elsewhere in the tree.

Fixes: eebfdc17cc6c ("backlight: Add TPS65217 WLED driver")
Cc: stable <redacted>     # 3.7
Cc: Matthias Kaehlcke <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>

Didn't I already ack this one?

Acked-by: Daniel Thompson <redacted>

On 20/11/17 10:45, Johan Hovold wrote:

Two framebuffer device-node names were looked up during probe, but were
only used as flags to indicate the presence of two framebuffer device.

Drop the unused framebuffer name along with a likewise unused device
pointer from the driver data, and update the platform data to pass in
booleans instead of the framebuffer strings. This allows us do drop the
node references acquired during probe, which would otherwise leak.

Note that there are no other in-kernel users of the modified
platform-data fields.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>

Acked-by: Daniel Thompson <redacted>

33 more quoted lines

On Mon, Nov 20, 2017 at 11:34:07AM +0000, Daniel Thompson wrote:

On 20/11/17 10:45, Johan Hovold wrote:

quoted

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

This would only cause trouble if the child node is missing while there
is an unrelated node named "backlight" elsewhere in the tree.

Fixes: eebfdc17cc6c ("backlight: Add TPS65217 WLED driver")
Cc: stable <redacted>     # 3.7
Cc: Matthias Kaehlcke <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>

Didn't I already ack this one?

Indeed you did (same with the previous one). Sorry about forgetting to
add them to v2.

Acked-by: Daniel Thompson <redacted>

Thanks for the acks.

Johan

On Mon, 20 Nov 2017, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: stable <redacted>     # 3.10
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/as3711_bl.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Mon, 20 Nov 2017, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

To make things worse, the parent mfd node was also prematurely freed,
while the child backlight node was leaked.

Fixes: 47ec340cb8e2 ("mfd: max8925: Support dt for backlight")
Cc: stable <redacted>     # 3.9
Cc: Qing Xu <redacted>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/max8925_bl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Mon, 20 Nov 2017, Johan Hovold wrote:

Fix child-node lookup during probe, which ended up searching the whole
device tree depth-first starting at the parent rather than just matching
on its children.

This would only cause trouble if the child node is missing while there
is an unrelated node named "backlight" elsewhere in the tree.

Fixes: eebfdc17cc6c ("backlight: Add TPS65217 WLED driver")
Cc: stable <redacted>     # 3.7
Cc: Matthias Kaehlcke <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/tps65217_bl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

On Mon, 20 Nov 2017, Johan Hovold wrote:

Two framebuffer device-node names were looked up during probe, but were
only used as flags to indicate the presence of two framebuffer device.

Drop the unused framebuffer name along with a likewise unused device
pointer from the driver data, and update the platform data to pass in
booleans instead of the framebuffer strings. This allows us do drop the
node references acquired during probe, which would otherwise leak.

Note that there are no other in-kernel users of the modified
platform-data fields.

Fixes: 59eb2b5e57ea ("drivers/video/backlight/as3711_bl.c: add OF support")
Cc: Guennadi Liakhovetski <redacted>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/video/backlight/as3711_bl.c | 12 ++++++------
 include/linux/mfd/as3711.h          |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

Applied, thanks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Hi Lee,

On Mon, Nov 20, 2017 at 11:45:43AM +0100, Johan Hovold wrote:

A number of drivers have been using the wrong OF helper when doing child-node
lookups during probe. This meant that they were doing tree-wide searches rather
than matching on child nodes and that the parent node could end up being
prematurely freed.

Johan Hovold (4):
  backlight: as3711_bl: fix device-tree node lookup
  backlight: max8925_bl: fix device-tree node lookup
  backlight: tps65217_bl: fix device-tree node lookup
  backlight: as3711_bl: fix device-tree node leaks

It appears these four have been sitting in your for-backlight-fixes 
branch for half a year now without being forwarded to Linus.

Don't get many backlight fixes these days, huh? ;)

Thanks,
Johan

On Fri, 11 May 2018, Johan Hovold wrote:

Hi Lee,

On Mon, Nov 20, 2017 at 11:45:43AM +0100, Johan Hovold wrote:

quoted

A number of drivers have been using the wrong OF helper when doing child-node
lookups during probe. This meant that they were doing tree-wide searches rather
than matching on child nodes and that the parent node could end up being
prematurely freed.

quoted

Johan Hovold (4):
  backlight: as3711_bl: fix device-tree node lookup
  backlight: max8925_bl: fix device-tree node lookup
  backlight: tps65217_bl: fix device-tree node lookup
  backlight: as3711_bl: fix device-tree node leaks

It appears these four have been sitting in your for-backlight-fixes 
branch for half a year now without being forwarded to Linus.

Don't get many backlight fixes these days, huh? ;)

Ha!  To be honest I think those patches are the only 4 -fixes I've
*ever* had for Backlight. :)

I have pulled them into the main branch, so they won't be forgotten
again.  Sorry for the delay.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog