Re: [PATCH v2 1/4] hwmon: chipcap2: fix channels in humidity alarm notifications

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unaligned memory access when casting byte arrays to `__be16 *`
- [High] Incorrect endianness conversion for SMBus word write breaks on Big-Endian architectures
--

--- Patch [1]: [PATCH v2 1/4] hwmon: chipcap2: fix channels in humidity alarm notifications ---
commit e05e5dc803c6cf895cd85ba5388bb75fbd791d04
Author: Javier Carrasco <javier.carrasco.cruz@gmail.com>

hwmon: chipcap2: fix channels in humidity alarm notifications

hwmon_notify_event() expects the channel number as its last argument,
taken into account with the type parameter that it is a humidity sensor
type. Given that this device only provides one humidity channel, 0 must
be passed. The custom construct to enumerate the channels makes wrong
assumptions by listing all types together (temperature and humidity).

Remove the custom channel enumeration and pass the right channel to
hwmon_notify_event() for hwmon_humidity_min_alarm and
hwmon_humidity_max_alarm.

> diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
> index 086571d556b7e..9bef767b589ee 100644
> --- a/drivers/hwmon/chipcap2.c
> +++ b/drivers/hwmon/chipcap2.c
> @@ -92,11 +92,6 @@ struct cc2_data {
>  	bool process_irqs;
>  };
>  
> -enum cc2_chan_addr {
> -	CC2_CHAN_TEMP = 0,
> -	CC2_CHAN_HUMIDITY,
> -};
> -
>  /* %RH as a per cent mille from a register value */
>  static long cc2_rh_convert(u16 data)
>  {
[ ... ]
> @@ -499,7 +494,7 @@ static irqreturn_t cc2_low_interrupt(int irq, void *data)
>  
>  	if (cc2->process_irqs) {
>  		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
> -				   hwmon_humidity_min_alarm, CC2_CHAN_HUMIDITY);
> +				   hwmon_humidity_min_alarm, 0);
>  		cc2->rh_alarm.low_alarm = true;
>  	}
>  
> @@ -512,7 +507,7 @@ static irqreturn_t cc2_high_interrupt(int irq, void *data)
>  
>  	if (cc2->process_irqs) {
>  		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
> -				   hwmon_humidity_max_alarm, CC2_CHAN_HUMIDITY);
> +				   hwmon_humidity_max_alarm, 0);
>  		cc2->rh_alarm.high_alarm = true;
>  	}

While reviewing this patch, I noticed a couple of pre-existing issues in
drivers/hwmon/chipcap2.c that you might want to look into.

drivers/hwmon/chipcap2.c:cc2_write_reg() {
...
	cpu_to_be16s(&val);
	ret = i2c_smbus_write_word_data(data->client, reg, val);
...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this endianness conversion
work correctly on Big-Endian architectures?

The SMBus protocol always transmits the least-significant byte first.
On a Little-Endian CPU, cpu_to_be16s() correctly swaps the bytes in memory,
causing SMBus to send the high byte first, matching the Big-Endian device's
expectations.

However, on a Big-Endian CPU, cpu_to_be16s() is a no-op, so the SMBus core
extracts the actual least-significant byte and sends it first, resulting in
Little-Endian transmission over the wire. Should this use
i2c_smbus_write_word_swapped() instead to ensure writes to device
configuration aren't corrupted?


drivers/hwmon/chipcap2.c:cc2_read_reg() {
...
	ret = i2c_master_recv(data->client, buf, CC2_EEPROM_DATA_LEN);
	if (ret != CC2_EEPROM_DATA_LEN)
		return ret < 0 ? ret : -EIO;

	*val = be16_to_cpup((__be16 *)&buf[1]);
...
}

[Severity: High]
This isn't a bug introduced by this patch, but does casting the unaligned
address &buf[1] to a 16-bit pointer cause an unaligned memory dereference?

Dereferencing an unaligned pointer using be16_to_cpup() can cause an
alignment fault and kernel oops on architectures that strictly enforce
memory alignment (e.g., SPARC or older ARM). This also happens in
cc2_data_fetch() when casting &data[2] to a 16-bit pointer:

drivers/hwmon/chipcap2.c:cc2_data_fetch() {
...
	switch (type) {
	case hwmon_humidity:
		*val = cc2_rh_convert(be16_to_cpup((__be16 *)&data[0]));
		break;
	case hwmon_temp:
		*val = cc2_temp_convert(be16_to_cpup((__be16 *)&data[2]));
...
}

Is there a safer way to parse these bytes without casting potentially
unaligned byte array addresses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823-chipcap2_locks-v2-0-6a26c8e9e2fc@gmail.com?part=1