Re: [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `in_min_raw` cache for the inactive voltage monitoring channel is not initialized from the hardware EEPROM at boot, leading to the hardware undervoltage limit being clobbered with 0 when the channel is enabled.
- [High] Uninitialized stack variable `struct clk_init_data init` is passed to the Common Clock Framework, leading to undefined behavior and potential panics.
--

--- Patch [3]: [PATCH 3/3] hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt ---
commit b52e0acebe8052dfb365a719ef77509a10b27e3a
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (ltc4282) Fix parsing adi,current-limit-sense-microvolt

ltc4282_parse_dt() evaluates the wrong variable when parsing the current
limit.

> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
> index bb1bcb3690163..b1675dc5b3c7f 100644
> --- a/drivers/hwmon/ltc4282.c
> +++ b/drivers/hwmon/ltc4282.c
> @@ -1394,7 +1394,7 @@ static int ltc4282_setup(struct ltc4282_state *st, struct device *dev)
>  	if (!ret) {
>  		int reg_val;
>  
> -		switch (val) {
> +		switch (st->vsense_max) {
>  		case 12500:
>  			reg_val = 0;
>  			break;

[Severity: High]
This isn't a bug introduced by this patch, but the in_min_raw cache for the
inactive voltage monitoring channel does not appear to be initialized from
hardware defaults during boot:

drivers/hwmon/ltc4282.c:ltc4282_get_defaults() {
...
	if (!st->in0_1_cache[LTC4282_CHAN_VSOURCE].en) {
		st->in0_1_cache[LTC4282_CHAN_VDD].en = true;
		return regmap_read(st->map, LTC4282_VSOURCE_MAX,
				   &st->in0_1_cache[LTC4282_CHAN_VSOURCE].in_max_raw);
	}

	return regmap_read(st->map, LTC4282_VSOURCE_MAX,
			   &st->in0_1_cache[LTC4282_CHAN_VDD].in_max_raw);
}

If a user later enables this inactive channel via sysfs, ltc4282_cache_sync()
writes the uninitialized in_min_raw (0) back to the hardware register:

drivers/hwmon/ltc4282.c:ltc4282_cache_sync() {
...
	ret = regmap_write(st->map, LTC4282_VSOURCE_MIN,
			   st->in0_1_cache[channel].in_min_raw);
...
}

Does this silently clobber and disable the hardware undervoltage limit?


[Severity: High]
This is a pre-existing issue, but I also noticed a potential problem with
uninitialized stack variables passed to the common clock framework:

drivers/hwmon/ltc4282.c:ltc428_clk_provider_setup() {
	struct clk_init_data init;
	int ret;

	if (!IS_ENABLED(CONFIG_COMMON_CLK))
		return 0;

	init.name =  devm_kasprintf(dev, GFP_KERNEL, "%s-clk",
				    fwnode_get_name(dev_fwnode(dev)));
...
	init.ops = &ltc4282_ops;
...
}

Since init is not zero-initialized, fields like init.num_parents and
init.parent_names will contain stack garbage. When devm_clk_hw_register()
evaluates init.num_parents, could it attempt to dereference the uninitialized
init.parent_names pointer and cause a panic during driver probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805005721.1566034-1-linux@roeck-us.net?part=3