Re: [PATCH] hwmon (ina2xx) Fix NULL id pointer in probe()

[Guenter Roeck]

On Fri, Nov 09, 2018 at 04:42:14PM -0800, Nicolin Chen wrote:
> When using DT configurations, the id pointer might turn out to
> be NULL. Then the driver encounters NULL pointer access:
> 
>   Unable to handle kernel read from unreadable memory at vaddr 0018
>   [...]
>   PC is at ina2xx_probe+0x114/0x200
>   LR is at ina2xx_probe+0x10c/0x200
>   [...]
>   Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b
> 
> The reason is that i2c core returns the id pointer by matching
> id_table with client->name, while the client->name is actually
> using the name from the first string in the DT compatible list,
> not the best one. So i2c core would fail to match the id_table
> if the best matched compatible string isn't the first one, and
> then would return a NULL id pointer.
> 
> This probably should be fixed in i2c core. But it doesn't hurt
> to make the driver robust. So this patch fixes it by using the
> "chip" that's added to unify both DT and non-DT configurations.
> 
> Additionally, since id pointer could be null, so as id->name:
>   ina2xx 10-0047: power monitor (null) (Rshunt = 1000 uOhm)
>   ina2xx 10-0048: power monitor (null) (Rshunt = 1 uOhm)
> 
> So this patch also fixes NULL name pointer, using client->name
> to play safe and to align with hwmon->name.
> 
> Fixes: bd0ddd4d0883 ("hwmon: (ina2xx) Add OF device ID table")
> Signed-off-by: Nicolin Chen 

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/ina2xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
> index 71d3445ba869..c2252cf452f5 100644
> --- a/drivers/hwmon/ina2xx.c
> +++ b/drivers/hwmon/ina2xx.c
> @@ -491,7 +491,7 @@ static int ina2xx_probe(struct i2c_client *client,
>   }
>  
>   data->groups[group++] = _group;
> - if (id->driver_data == ina226)
> + if (chip == ina226)
>   data->groups[group++] = _group;
>  
>   hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
> @@ -500,7 +500,7 @@ static int ina2xx_probe(struct i2c_client *client,
>   return PTR_ERR(hwmon_dev);
>  
>   dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
> -  id->name, data->rshunt);
> +  client->name, data->rshunt);
>  
>   return 0;
>  }
> -- 
> 2.17.1
> 

[PATCH] hwmon (ina2xx) Fix NULL id pointer in probe()

[Nicolin Chen]

When using DT configurations, the id pointer might turn out to
be NULL. Then the driver encounters NULL pointer access:

  Unable to handle kernel read from unreadable memory at vaddr 0018
  [...]
  PC is at ina2xx_probe+0x114/0x200
  LR is at ina2xx_probe+0x10c/0x200
  [...]
  Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b

The reason is that i2c core returns the id pointer by matching
id_table with client->name, while the client->name is actually
using the name from the first string in the DT compatible list,
not the best one. So i2c core would fail to match the id_table
if the best matched compatible string isn't the first one, and
then would return a NULL id pointer.

This probably should be fixed in i2c core. But it doesn't hurt
to make the driver robust. So this patch fixes it by using the
"chip" that's added to unify both DT and non-DT configurations.

Additionally, since id pointer could be null, so as id->name:
  ina2xx 10-0047: power monitor (null) (Rshunt = 1000 uOhm)
  ina2xx 10-0048: power monitor (null) (Rshunt = 1 uOhm)

So this patch also fixes NULL name pointer, using client->name
to play safe and to align with hwmon->name.

Fixes: bd0ddd4d0883 ("hwmon: (ina2xx) Add OF device ID table")
Signed-off-by: Nicolin Chen 
---
 drivers/hwmon/ina2xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/ina2xx.c b/drivers/hwmon/ina2xx.c
index 71d3445ba869..c2252cf452f5 100644
--- a/drivers/hwmon/ina2xx.c
+++ b/drivers/hwmon/ina2xx.c
@@ -491,7 +491,7 @@ static int ina2xx_probe(struct i2c_client *client,
}
 
data->groups[group++] = _group;
-   if (id->driver_data == ina226)
+   if (chip == ina226)
data->groups[group++] = _group;
 
hwmon_dev = devm_hwmon_device_register_with_groups(dev, client->name,
@@ -500,7 +500,7 @@ static int ina2xx_probe(struct i2c_client *client,
return PTR_ERR(hwmon_dev);
 
dev_info(dev, "power monitor %s (Rshunt = %li uOhm)\n",
-id->name, data->rshunt);
+client->name, data->rshunt);
 
return 0;
 }
-- 
2.17.1