Re: [PATCH 2/2] hwmon: (chipcap2) Add support for label

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Data race on shared state between threaded interrupt handlers and sysfs context, violating hwmon subsystem guidelines.
--

commit a4752e881c74f908f48e460d6cd5322c3eb88341
Author: Flaviu Nistor <flaviu.nistor@gmail.com>

hwmon: (chipcap2) Add support for label

This commit adds support for the label sysfs attribute to the chipcap2 hwmon
device. This allows for easier identification of individual sensors via the
device tree.

> diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
> index 4aecf463180fd..086571d556b7e 100644
> --- a/drivers/hwmon/chipcap2.c
> +++ b/drivers/hwmon/chipcap2.c
[ ... ]
> @@ -552,6 +559,16 @@ static int cc2_humidity_max_alarm_status(struct cc2_data *data, long *val)
>  	return 0;
>  }
>  

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this driver I
noticed a potential data race. Does the driver access and modify shared
variables (rh_alarm.low_alarm) concurrently from both sysfs read paths and
threaded interrupt handlers without locking?

In the sysfs process context, the flag is evaluated and cleared without locks:

drivers/hwmon/chipcap2.c:cc2_humidity_min_alarm_status() {
    ...
    if (data->rh_alarm.low_alarm) {
        *val = (measurement < min_hyst) ? 1 : 0;
        data->rh_alarm.low_alarm = *val;
    } else {
        *val = 0;
    }
    ...
}

Concurrently, the threaded interrupt handler sets the flag:

drivers/hwmon/chipcap2.c:cc2_low_interrupt() {
    ...
    if (cc2->process_irqs) {
        hwmon_notify_event(cc2->hwmon, hwmon_humidity,
                           hwmon_humidity_min_alarm, CC2_CHAN_HUMIDITY);
        cc2->rh_alarm.low_alarm = true;
    }
    ...
}

The hwmon subsystem guidelines mandate that drivers must implement locking
required for interrupt handling, specifically recommending the use of
hwmon_lock() and hwmon_unlock() to serialize operations. Can these paths
race and cause state corruption or lost alarm events?

> +static int cc2_read_string(struct device *dev, enum hwmon_sensor_types type,
> +			   u32 attr, int channel, const char **str)
> +{
> +	struct cc2_data *data = dev_get_drvdata(dev);
> +
> +	*str = data->label;
> +
> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622122200.14245-1-flaviu.nistor@gmail.com?part=2