Re: [PATCH 2/2] hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: undefined behavior in GENMASK() due to unchecked i2c_get_match_data()
--

commit 5632ba367123600a6ee38c612901f594c35ee0df
Author: Luna Hart <lhart.opensource@witekio.com>

hwmon: (tps38900x) Add driver for TI TPS389004/TPS389006/TPS389008

This commit adds support for the TI TPS389004/TPS389006/TPS389008 I2C
voltage supervisors and monitors. It allows enabling ADC monitoring and
reading back the measured voltage in millivolts via sysfs.

> diff --git a/drivers/hwmon/tps38900x.c b/drivers/hwmon/tps38900x.c
> index 0000000000000..659d61b1cb55c 100644
> --- /dev/null
> +++ b/drivers/hwmon/tps38900x.c
[ ... ]
> +static int tps38900x_parse_dt_channels(struct device *dev,
> +				       struct tps38900x_data *priv)
> +{
[ ... ]
> +	// When no channels are explicitly described in the DT, expose all of them.
> +	if (!priv->channel_present)
> +		priv->channel_present = GENMASK(priv->num_channels - 1, 0);

[Severity: Medium]
Could this result in undefined behavior in tps38900x_parse_dt_channels()
in drivers/hwmon/tps38900x.c? If the driver is bound via sysfs
driver_override using an I2C device name not present in the driver's ID
table, num_channels would be 0. This evaluates to GENMASK(-1, 0), which
performs a bitwise shift by BITS_PER_LONG bits.

> +
> +	return 0;
> +}
> +
> +static int tps38900x_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct tps38900x_data *priv;
> +	struct device *hwmon_dev;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -ENODEV;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->num_channels = (uintptr_t)i2c_get_match_data(client);

[Severity: Medium]
Does this code in tps38900x_probe() in drivers/hwmon/tps38900x.c allow 0 to
be stored as the number of channels? If i2c_get_match_data() returns NULL
when a user binds the driver via sysfs driver_override, num_channels becomes
0 without being checked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-lhart-dev-tps38900x-v1-0-7045c5894152@witekio.com?part=2