Messages in this thread | | | From | Benjamin Bara <> | Subject | Re: [PATCH v2] regulator: da9063: fix null pointer deref with partial DT config | Date | Thu, 22 Jun 2023 17:35:24 +0200 |
| |
Hi,
On Thu, 22 Jun 2023 at 17:11, Martin Fuzzey <martin.fuzzey@flowbird.group> wrote: > 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, ®l->desc, > &config); > -- > 2.25.1
Thank you!
As this is the same as I did in my patch[1], which I tested by removing some LDO DT nodes, feel free to add: Tested-by: Benjamin Bara <benjamin.bara@skidata.com>
br, Benjamin
[1] https://lore.kernel.org/lkml/20230419-dynamic-vmon-v4-1-4d3734e62ada@skidata.com/
| |