Re: [PATCH AUTOSEL 6.18] hwmon: (asus-ec-sensors) add ROG MAXIMUS Z790 EXTREME

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Spurious EC register reads due to incorrect bank check condition in asus_ec_block_read().
- [High] The return value of asus_ec_hwmon_add_chan_info() is ignored, leading to a NULL pointer dereference in the hwmon core if allocation fails.
--

commit 751ec7ce900f71bbf414765f09e4deeff7e667fe
Author: Brian Downey <bdowne01@gmail.com>

hwmon: (asus-ec-sensors) add ROG MAXIMUS Z790 EXTREME

Add support for ROG MAXIMUS Z790 EXTREME.

Link: https://lore.kernel.org/r/20260608060855.40469-1-eugene.shalygin@gmail.com

> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index 34a8f6b834c97..bd89ffbe25897 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c

[ ... ]

> @@ -470,6 +476,13 @@ static const struct ec_board_info board_info_maximus_z690_formula = {
>  	.family = family_intel_600_series,
>  };
>  
> +static const struct ec_board_info board_info_maximus_z790_extreme = {
> +	.sensors = SENSOR_TEMP_T_SENSOR | SENSOR_TEMP_VRM |
> +		SENSOR_SET_TEMP_WATER | SENSOR_FAN_WATER_FLOW,
> +	.mutex_path = ASUS_HW_ACCESS_MUTEX_RMTW_ASMX,
> +	.family = family_intel_700_series,
> +};
> +

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the new board
support, I noticed a pre-existing issue in asus_ec_block_read() that could
affect hardware interaction on boards with sensors spanning multiple banks.

Could this condition cause spurious reads on unintended EC registers?

drivers/hwmon/asus-ec-sensors.c:asus_ec_block_read() {
		for (ireg = 0; ireg < ec->nr_registers; ireg++) {
			reg_bank = register_bank(ec->registers[ireg]);
			if (reg_bank < bank) {
				continue;
			}
			ec_read(register_index(ec->registers[ireg]),
				ec->read_buffer + ireg);
		}
}

The loop uses if (reg_bank < bank) to skip registers from previously
processed banks, but it appears to not skip registers from future banks where
reg_bank > bank.

Since the read uses the current hardware bank, could this result in unintended
reads that might trigger unexpected hardware side-effects?


[Severity: High]
This is a pre-existing issue, but I also noticed a potential missing error
handling path in the driver's probe function.

Does this loop in asus_ec_probe() silently discard the error return value from
asus_ec_hwmon_add_chan_info()?

drivers/hwmon/asus-ec-sensors.c:asus_ec_probe() {
	for (type = 0; type < hwmon_max; ++type) {
		if (!nr_count[type])
			continue;

		asus_ec_hwmon_add_chan_info(asus_ec_hwmon_chan, dev,
					     nr_count[type], type,
					     hwmon_attributes[type]);
		*ptr_asus_ec_ci++ = asus_ec_hwmon_chan++;
	}
}

Because asus_ec_hwmon_add_chan_info() can return -ENOMEM if its memory
allocation fails, ignoring its return value could leave the config pointer in
asus_ec_hwmon_chan as NULL.

When this partially initialized structure is subsequently passed to
devm_hwmon_device_register_with_info(), the hwmon core will iterate over the
channel configuration in hwmon_num_channel_attrs().

Could this cause a NULL pointer dereference when the core iterates using
for (i = n = 0; info->config[i]; i++)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-412-sashal@kernel.org?part=1