[PATCH] regulator: Fix regulator_summary for deviceless consumers

[PATCH] regulator: Fix regulator_summary for deviceless consumers

[Leonard Crestez]

It is allowed to call regulator_get with a NULL dev argument
(_regulator_get explicitly checks for it) but this causes an error later
when printing /sys/kernel/debug/regulator_summary.

Fix this by explicitly handling "deviceless" consumers in the debugfs code.

This fixes errors like this:

root@leonard-imx6:~# cat /sys/kernel/debug/regulator/regulator_summary
Unable to handle kernel NULL pointer dereference at virtual address 0000015c
pgd = a8bd4000
[0000015c] *pgd=a8c15831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#2] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 673 Comm: cat Tainted: G      D         4.9.5-01680-g3d4992e #18
Hardware name: Freescale i.MX6 SoloLite (Device Tree)
task: a8e07700 task.stack: a8c2a000
PC is at regulator_summary_show_subtree+0x1b4/0x274
LR is at seq_puts+0x48/0x58
pc : [<8044eb90>]    lr : [<8021f82c>]    psr: 800e0013
sp : a8c2bdf0  ip : 00000000  fp : 80c52720
r10: 80c21fc0  r9 : 00000004  r8 : 80f253a4
r7 : a821601c  r6 : a874d400  r5 : a8216000  r4 : a8b1c000
r3 : 80c21fc0  r2 : 00000004  r1 : 80c52720  r0 : a8b1c000
Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 10c53c7d  Table: a8bd404a  DAC: 00000055
Process cat (pid: 673, stack limit = 0xa8c2a210)
Stack: (0xa8c2bdf0 to 0xa8c2c000)
bde0:                                     0000001e 80c167f0 00000001 00000002
be00: 00000000 00000000 00000004 0000001b 10624dd3 00000000 a80ca35c a8c2be64
be20: 8044ec50 a8190dac 00020000 a8216060 a8b1c000 8044ec50 a8c2bf80 00000001
be40: 00000000 a8c2be98 00020000 8044ec78 a8216060 804b942c 80c524f8 a8c2bf80
be60: 00000001 a80ca35c a82161ac 00000000 a8b1c000 a87cd240 a87cd240 8044dc6c
be80: a8b1c000 8021efc4 a8bd5db8 76e73000 a8b1c030 00000000 00000000 00000000
bea0: a8c2bf80 80177644 a60017f8 8021ee2c a87cd240 76e73000 00020000 a8c2bf80
bec0: 80a48d9c 00000000 00020000 80356080 a8bd3bc0 00000001 00000800 a87cd240
bee0: 80356034 00020000 a8c2bf80 00000000 00020000 801fbf4c 76e72004 a8c2bfb0
bf00: 00002710 76fca000 76faed14 801012b0 00000000 80114820 00022000 00022000
bf20: a8c2bf8c 00000003 00000000 a8b488c0 76e72000 00000022 00000073 801e44a4
bf40: 00076e72 a87cd240 76e73000 00020000 a8c2bf80 00000000 00020000 801fcd08
bf60: 00022000 00000003 a87cd240 a87cd240 00000000 00000000 76e73000 801fdb00
bf80: 00000000 00000000 00000000 00020000 00020000 76e73000 00000003 80107724
bfa0: a8c2a000 80107580 00020000 00020000 00000003 76e73000 00020000 0002a3fc
bfc0: 00020000 00020000 76e73000 00000003 7fffe000 00000000 76ffe000 00020000
bfe0: 00000000 7ea10b64 00014b14 76f52770 600e0010 00000003 00000000 00000000
[<8044eb90>] (regulator_summary_show_subtree) from [<8044ec78>] (regulator_summary_show_roots+0x28/0x30)
[<8044ec78>] (regulator_summary_show_roots) from [<804b942c>] (class_for_each_device+0x4c/0xb4)
[<804b942c>] (class_for_each_device) from [<8044dc6c>] (regulator_summary_show+0x3c/0x48)
[<8044dc6c>] (regulator_summary_show) from [<8021efc4>] (seq_read+0x198/0x4a0)
[<8021efc4>] (seq_read) from [<80356080>] (full_proxy_read+0x4c/0x6c)
[<80356080>] (full_proxy_read) from [<801fbf4c>] (__vfs_read+0x1c/0x10c)
[<801fbf4c>] (__vfs_read) from [<801fcd08>] (vfs_read+0x8c/0x118)
[<801fcd08>] (vfs_read) from [<801fdb00>] (SyS_read+0x3c/0x90)
[<801fdb00>] (SyS_read) from [<80107580>] (ret_fast_syscall+0x0/0x34)
Code: e59f80c4 e1a0100b e59d2018 e1a00004 (e59ce15c)
---[ end trace eca2c2e6d835da26 ]---

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Some quick grepping only finds a very small number of users for
regulator_get(NULL, ...), it might make more sense to simply require
always passing a device pointer. I found this because there was a patch
in the freescale imx tree which did this and in our case it was easier
to just fix the consumer.

Here are the current obvious upstream users:
$ git grep 'regulator_get(NULL'
arch/arm/mach-davinci/da850.c:	cvdd = regulator_get(NULL, "cvdd");
arch/arm/mach-pxa/em-x270.c:	em_x270_usb_ldo = regulator_get(NULL, "vcc usb");
drivers/cpufreq/pxa2xx-cpufreq.c:	vcc_core = regulator_get(NULL, "vcc_core");
drivers/cpufreq/s3c2416-cpufreq.c:	s3c_freq->vddarm = regulator_get(NULL, "vddarm");
drivers/cpufreq/s3c64xx-cpufreq.c:	vddarm = regulator_get(NULL, "vddarm");
drivers/cpufreq/s5pv210-cpufreq.c:	arm_regulator = regulator_get(NULL, "vddarm");
drivers/cpufreq/s5pv210-cpufreq.c:	int_regulator = regulator_get(NULL, "vddint");

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 04baac9..6631954 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4391,12 +4391,13 @@ static void regulator_summary_show_subtree(struct seq_file *s,
 	seq_puts(s, "\n");
 
 	list_for_each_entry(consumer, &rdev->consumer_list, list) {
-		if (consumer->dev->class == &regulator_class)
+		if (consumer->dev && consumer->dev->class == &regulator_class)
 			continue;
 
 		seq_printf(s, "%*s%-*s ",
 			   (level + 1) * 3 + 1, "",
-			   30 - (level + 1) * 3, dev_name(consumer->dev));
+			   30 - (level + 1) * 3,
+			   consumer->dev ? dev_name(consumer->dev) : "deviceless");
 
 		switch (rdev->desc->type) {
 		case REGULATOR_VOLTAGE:
-- 
2.7.4

Re: [PATCH] regulator: Fix regulator_summary for deviceless consumers

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Tue, Feb 14, 2017 at 05:31:03PM +0200, Leonard Crestez wrote:

> root@leonard-imx6:~# cat /sys/kernel/debug/regulator/regulator_summary
> Unable to handle kernel NULL pointer dereference at virtual address 0000015c
> pgd = a8bd4000
> [0000015c] *pgd=a8c15831, *pte=00000000, *ppte=00000000

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative then it's
usually better to pull out the relevant sections.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Applied "regulator: Fix regulator_summary for deviceless consumers" to the regulator tree

[Mark Brown]

The patch

   regulator: Fix regulator_summary for deviceless consumers

has been applied to the regulator tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From e42a46b6f52473661ad192f76a128a68fe301df4 Mon Sep 17 00:00:00 2001
From: Leonard Crestez <leonard.crestez@nxp.com>
Date: Tue, 14 Feb 2017 17:31:03 +0200
Subject: [PATCH] regulator: Fix regulator_summary for deviceless consumers

It is allowed to call regulator_get with a NULL dev argument
(_regulator_get explicitly checks for it) but this causes an error later
when printing /sys/kernel/debug/regulator_summary.

Fix this by explicitly handling "deviceless" consumers in the debugfs code.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/regulator/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 04baac9a165b..66319542baa6 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4391,12 +4391,13 @@ static void regulator_summary_show_subtree(struct seq_file *s,
 	seq_puts(s, "\n");
 
 	list_for_each_entry(consumer, &rdev->consumer_list, list) {
-		if (consumer->dev->class == &regulator_class)
+		if (consumer->dev && consumer->dev->class == &regulator_class)
 			continue;
 
 		seq_printf(s, "%*s%-*s ",
 			   (level + 1) * 3 + 1, "",
-			   30 - (level + 1) * 3, dev_name(consumer->dev));
+			   30 - (level + 1) * 3,
+			   consumer->dev ? dev_name(consumer->dev) : "deviceless");
 
 		switch (rdev->desc->type) {
 		case REGULATOR_VOLTAGE:
-- 
2.11.0