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

["Javier Carrasco" <javier.carrasco.cruz@gmail.com>]

On Sun Aug 23, 2026 at 8:11 PM CEST, sashiko-bot wrote:
> 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?

I will take a look at this and send another patch if it applies.

Best regards,
Javier