Re: [PATCH v2 3/3] power: supply: bq27xxx: add support for bq27z746

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential null pointer dereference in `bq27xxx_battery_i2c_probe()` when retrieving the I2C device ID. The driver relies on `i2c_client_get_device_id()` without checking its return value.
--

--- Patch [3]: [PATCH v2 3/3] power: supply: bq27xxx: add support for bq27z746 ---
commit a0624890a79f9b2398a034239df6144de2c581e9
Author: Henrik Grimler <[email protected]>

power: supply: bq27xxx: add support for bq27z746

bq27z746 is similar to bq27z561, but has additional battery protection
logic. The standard register map is a subset of the one found in
bq27z561.

> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
> index c4e7a9521d086..2a6b4534cb28a 100644
> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
> @@ -253,6 +253,7 @@ static const struct i2c_device_id bq27xxx_i2c_id_table[] = {
>  	{ .name = "bq27441", .driver_data = BQ27441 },
>  	{ .name = "bq27621", .driver_data = BQ27621 },
>  	{ .name = "bq27z561", .driver_data = BQ27Z561 },
> +	{ .name = "bq27z746", .driver_data = BQ27Z746 },

[Severity: High]
This is a pre-existing issue, but adding a new device id highlights a
potential null pointer dereference in bq27xxx_battery_i2c_probe().

If a system integrator provides a Device Tree containing a valid fallback
compatible string (for example: compatible = "ti,unsupported-chip",
"ti,bq27z746";), probe could be called with client->name set to
"unsupported-chip".

Since "unsupported-chip" is not in the I2C ID table,
i2c_client_get_device_id() will return NULL:

static int bq27xxx_battery_i2c_probe(struct i2c_client *client)
{
	const struct i2c_device_id *id = i2c_client_get_device_id(client);
	...
	name = devm_kasprintf(&client->dev, GFP_KERNEL, "%s-%ld", id->name, num);
	...
	di->chip = id->driver_data;
	...
}

Does this code cause a kernel panic when id->name and id->driver_data are
accessed if id is NULL? Would it be safer to explicitly check if id is
valid, or use i2c_get_match_data() to retrieve the driver_data?

>  	{ .name = "bq28z610", .driver_data = BQ28Z610 },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3