Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Martin Fuzzey <martin.fuzzey@flowbird.group>
To:     Mark Brown <broonie@kernel.org>
Cc:     Support Opensource <support.opensource@diasemi.com>,
        Benjamin Bara <benjamin.bara@skidata.com>,
        Adam Ward <DLG-Adam.Ward.opensource@dm.renesas.com>,
        linux-kernel@vger.kernel.org
Subject: [PATCH v2] regulator: da9063: fix null pointer deref with partial DT config
Date:   Thu, 22 Jun 2023 17:11:02 +0200
Message-Id: <20230622151146.1459250-1-martin.fuzzey@flowbird.group>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

When some of the da9063 regulators do not have corresponding DT nodes
a null pointer dereference occurs on boot:

[    1.559034] 8<--- cut here ---
[    1.564014] Unable to handle kernel NULL pointer dereference at virtual address 00000098 when read
[    1.578055] [00000098] *pgd=00000000
[    1.593575] Internal error: Oops: 5 [#1] SMP ARM
[    1.634870] PC is at da9063_regulator_probe+0x35c/0x788
[    1.647934] LR is at da9063_regulator_probe+0x2e8/0x788
[    2.073626]  da9063_regulator_probe from platform_probe+0x58/0xb8
[    2.079759]  platform_probe from really_probe+0xd8/0x3c0
[    2.085092]  really_probe from __driver_probe_device+0x94/0x1e8
[    2.091026]  __driver_probe_device from driver_probe_device+0x2c/0xd0
[    2.097479]  driver_probe_device from __device_attach_driver+0xa4/0x11c
[    2.104107]  __device_attach_driver from bus_for_each_drv+0x84/0xdc
[    2.110402]  bus_for_each_drv from __device_attach_async_helper+0xb0/0x110
[    2.117295]  __device_attach_async_helper from async_run_entry_fn+0x3c/0x158
[    2.124369]  async_run_entry_fn from process_one_work+0x1d4/0x3e4
[    2.130485]  process_one_work from worker_thread+0x30/0x520
[    2.136070]  worker_thread from kthread+0xdc/0xfc

This is because such regulators have no init_data causing the pointers
calculated in da9063_check_xvp_constraints() to be invalid.

Do not dereference them in this case.

Fixes: b8717a80e6ee ("regulator: da9063: implement setter for voltage monitoring")
Signed-off-by: Martin Fuzzey <martin.fuzzey@flowbird.group>
---

Changes since V1:
 -  Following review by Mark Brown avoid previous dereferences too.
    With the GCC versions I tried this didn't cause problems
    because it only takes the address ie
        &config->init_data->constraints
    doesn't fault if config->init_data is NULL (it would without the &)
    But this behaviour isn't guaranteed and other compilers or compiler
    versions could behave differently so completely avoid calling the
    function if config->init_data is NULL.

 drivers/regulator/da9063-regulator.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/da9063-regulator.c b/drivers/regulator/da9063-regulator.c
index c5dd77be558b..a0621665a6d2 100644
--- a/drivers/regulator/da9063-regulator.c
+++ b/drivers/regulator/da9063-regulator.c
@@ -1028,9 +1028,12 @@ static int da9063_regulator_probe(struct platform_device *pdev)
 			config.of_node = da9063_reg_matches[id].of_node;
 		config.regmap = da9063->regmap;
 
-		ret = da9063_check_xvp_constraints(&config);
-		if (ret)
-			return ret;
+		/* Checking constraints requires init_data from DT. */
+		if (config.init_data) {
+			ret = da9063_check_xvp_constraints(&config);
+			if (ret)
+				return ret;
+		}
 
 		regl->rdev = devm_regulator_register(&pdev->dev, &regl->desc,
 						     &config);
-- 
2.25.1