linux-kernel - Re: [BUG] Error applying setting, reverse things back on lot of devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201108170807.GA10914@qmqm.qmqm.pl>
Date:   Sun, 8 Nov 2020 18:08:07 +0100
From:   Michał Mirosław <mirq-linux@...e.qmqm.pl>
To:     Ahmad Fatoum <a.fatoum@...gutronix.de>
Cc:     Corentin Labbe <clabbe.montjoie@...il.com>, lgirdwood@...il.com,
        mripard@...nel.org, linux-kernel@...r.kernel.org, wens@...e.org,
        broonie@...nel.org, linux-arm-kernel@...ts.infradead.org,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        Pengutronix Kernel Team <kernel@...gutronix.de>
Subject: Re: [BUG] Error applying setting, reverse things back on lot of
 devices

On Thu, Nov 05, 2020 at 10:11:30AM +0100, Ahmad Fatoum wrote:
> Hello,
> 
> On 11/5/20 3:57 AM, Michał Mirosław wrote:
> >>> Can you catch debug logs for the bootup in question? I'm not sure what's
> >>> the failure mode in your case. I guess this is not a bypassed regulator?
> >>
> >> Boot up with v5.10-rc2 + your cf1ad559a2 ("regulator: defer probe when trying
> >> to get voltage from unresolved supply") hangs:
> >>
> >> [    1.151489] stm32f7-i2c 40015000.i2c: STM32F7 I2C-0 bus adapter
> >> [    1.180698] stpmic1 1-0033: PMIC Chip Version: 0x10
> >> [    1.189526] vddcore: supplied by regulator-dummy
> >> [    1.195633] vdd_ddr: supplied by regulator-dummy
> >> [    1.201672] vdd: supplied by regulator-dummy
> >> [    1.207452] v3v3: supplied by 5V2
> >> [    1.211997] v1v8_audio: supplied by v3v3
> >> [    1.218036] v3v3_hdmi: supplied by 5V2
> >> [    1.223626] vtt_ddr: supplied by regulator-dummy
> >> [    1.227107] vdd_usb: supplied by regulator-dummy
> >> [    1.234532] vdda: supplied by 5V2
> >> [    1.239497] v1v2_hdmi: supplied by v3v3
> > [...]
> > 
> > Can you try with the patches I just sent and with debug logs enabled?
> > 
> > The first one just plugs a memory leak, but if there is some state
> > changed/saved in the rdev->constraints (can't find that code, though),
> > this might prevent it from being overwritten.
> > 
> > The second patch will just tell us if you hit the early resolve case.
> 
> Problem still persists. Early resolve case not hit:
[...]
> [    1.594492] vref_ddr: at 500 mV, enabled
> [    1.597047] edt_ft5x06 0-0038: touchscreen probe failed
> [    1.597211] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking up vref_ddr-supply from device tree
> [    1.612406] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking up vref_ddr-supply property in node /soc/i2c@...02000/stpmic@...regulators failed
> 
>   [ snip - continues many times ]
> 
> [    6.699244] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking up vref_ddr-supply property in node /soc/i2c@...02000/stpmic@...regulators failed
> [    6.713312] stpmic1-regulator 5c002000.i2c:stpmic@33:regulators: Looking up vref_ddr-supply from device tree

It seems that final regulator_resolve_supply() is spinning recursively.
Is the regulator name the same as its supply_name? Can you try the patch
below to verify this?

Best Regards
Michał Mirosław

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index c84e3b0b63de..983a4bd3e98c 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1798,6 +1798,8 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 	if (rdev->supply)
 		return 0;
 
+	dev_dbg(dev, "Resolving supply %s for %s\n", rdev->supply_name, rdev->desc->name);
+
 	r = regulator_dev_lookup(dev, rdev->supply_name);
 	if (IS_ERR(r)) {
 		ret = PTR_ERR(r);
@@ -1816,6 +1818,12 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
 		}
 	}
 
+	if (r == rdev) {
+		dev_err(dev, "Supply for %s (%s) resolved to itself\n",
+			rdev->desc->name, rdev->supply_name);
+		return -EINVAL;
+	}
+
 	/*
 	 * If the supply's parent device is not the same as the
 	 * regulator's parent device, then ensure the parent device