Re: [PATCH] max8907: fix use of possibly NULL idata

Re: [PATCH] max8907: fix use of possibly NULL idata

[Laxman Dewangan]

On Thursday 23 August 2012 11:55 PM, Stephen Warren wrote:
> On 08/23/2012 12:19 PM, Stephen Warren wrote:
> Laxman,
>
> The TPS6586x driver avoids this NULL-dereference issue by simply not
> registering any regulators when idata is NULL. See
> drivers/mfd/tps6586x.c:tps6586x_parse_dt():
>
>>          for (i = 0, j = 0; i<  num&&  j<  count; i++) {
>>                  struct regulator_init_data *reg_idata;
>>
>>                  if (!tps6586x_matches[i].init_data)
>>                          continue;
> If I interpreted Mark Brown correctly, this isn't correct; all the
> regulators within the chip should always be registered, just without any
> user-supplied constraints. Assuming I didn't misinterpret Mark, can you
> please fix the TPS6586x driver to always register the regulators, and
> apply the fix below. Could you please check the TPS65911 driver and see
> what the status is there too? Thanks very much!

Sure, I will check it.

[PATCH] max8907: fix use of possibly NULL idata

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

If a regulator is not used by a board, it's quite legitimate not to
provide platform data or a device tree node to configure it (i.e.
regulator_init_data). In that case, during MAX8907 regulator's
probe(), the idata variable will be NULL for that regulator. Prevent
dereferincing it.

If the MBATT regulator's init_data is not specified, or no name was
specified in the constraints, the regulator will be named based on
the regulator descriptor, so initialize mbatt_rail_name from there.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 drivers/regulator/max8907-regulator.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/regulator/max8907-regulator.c b/drivers/regulator/max8907-regulator.c
index 3a5104f..e97af8e 100644
--- a/drivers/regulator/max8907-regulator.c
+++ b/drivers/regulator/max8907-regulator.c
@@ -323,7 +323,10 @@ static __devinit int max8907_regulator_probe(struct platform_device *pdev)
 
 		switch (pmic->desc[i].id) {
 		case MAX8907_MBATT:
-			mbatt_rail_name = idata->constraints.name;
+			if (idata && idata->constraints.name)
+				mbatt_rail_name = idata->constraints.name;
+			else
+				mbatt_rail_name = pmic->desc[i].name;
 			break;
 		case MAX8907_BBAT:
 		case MAX8907_SDBY:
-- 
1.7.0.4

Re: [PATCH] max8907: fix use of possibly NULL idata

[Stephen Warren]

On 08/23/2012 12:19 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> If a regulator is not used by a board, it's quite legitimate not to
> provide platform data or a device tree node to configure it (i.e.
> regulator_init_data). In that case, during MAX8907 regulator's
> probe(), the idata variable will be NULL for that regulator. Prevent
> dereferincing it.
> 
> If the MBATT regulator's init_data is not specified, or no name was
> specified in the constraints, the regulator will be named based on
> the regulator descriptor, so initialize mbatt_rail_name from there.

Laxman,

The TPS6586x driver avoids this NULL-dereference issue by simply not
registering any regulators when idata is NULL. See
drivers/mfd/tps6586x.c:tps6586x_parse_dt():

>         for (i = 0, j = 0; i < num && j < count; i++) {
>                 struct regulator_init_data *reg_idata;
> 
>                 if (!tps6586x_matches[i].init_data)
>                         continue;

If I interpreted Mark Brown correctly, this isn't correct; all the
regulators within the chip should always be registered, just without any
user-supplied constraints. Assuming I didn't misinterpret Mark, can you
please fix the TPS6586x driver to always register the regulators, and
apply the fix below. Could you please check the TPS65911 driver and see
what the status is there too? Thanks very much!

Re: [PATCH] max8907: fix use of possibly NULL idata

[Mark Brown]

On Thu, Aug 23, 2012 at 12:19:18PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> If a regulator is not used by a board, it's quite legitimate not to
> provide platform data or a device tree node to configure it (i.e.
> regulator_init_data). In that case, during MAX8907 regulator's
> probe(), the idata variable will be NULL for that regulator. Prevent
> dereferincing it.

Applied, thanks.  Please do remember to use subject lines which match up
with the subsystem.

Re: [PATCH] max8907: fix use of possibly NULL idata

[Mark Brown]

On Thu, Aug 23, 2012 at 12:25:19PM -0600, Stephen Warren wrote:

> If I interpreted Mark Brown correctly, this isn't correct; all the
> regulators within the chip should always be registered, just without any
> user-supplied constraints. Assuming I didn't misinterpret Mark, can you

That's what I meant, yes.