Re: [PATCH v2 2/4] hwmon: chipcap2: fix IRQ teardown ordering

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

On Sun Aug 23, 2026 at 8:12 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] Registering the hwmon device before requesting IRQs exposes the device to userspace before its resources are fully initialized, introducing a probe race.
>
> Pre-existing issues:
> - [High] Incorrect endianness conversion on big-endian hosts for SMBus word writes results in writing LSB first instead of MSB first.
> - [High] Dereferencing a potentially unaligned byte array pointer using `be16_to_cpup()` can cause kernel panics on architectures with strict memory alignment requirements.
> --
>
> --- Patch [2]: [PATCH v2 2/4] hwmon: chipcap2: fix IRQ teardown ordering ---
> Note: The standard review format has been altered and direct code quotes omitted to comply with recitation restrictions.
>
> commit 28a819427451e0b54e63443c10cc5e1979c146bb
> Author: Javier Carrasco <javier.carrasco.cruz@gmail.com>
> hwmon: chipcap2: fix IRQ teardown ordering
>
> This commit separates IRQ discovery from IRQ registration in the chipcap2
> driver. It ensures IRQs are requested after the hwmon device is registered to
> fix devres teardown ordering issues.
>
> [Severity: Medium]
> In cc2_probe(), the hwmon device is now registered using
> devm_hwmon_device_register_with_info() before cc2_request_irqs() is called.
> Does exposing the sysfs attributes before the IRQs are requested create a probe
> race?
>
> If userspace applications interact with the sysfs interface immediately after
> registration, could the read fail with a -ETIMEDOUT error because the device
> is waiting for an IRQ handler that has not yet been registered?
>

I am not sure what to do with this, as this patch was actually meant to
avoid the opposite issue (IRQs still available when the hwmon device is
not) which is worse because access to freed resources might arise.

> [Severity: High]
> This is a pre-existing issue, but looking at cc2_write_reg(), the code calls
> cpu_to_be16s() on the value before passing it to i2c_smbus_write_word_data().
> Since the SMBus API internally expects a host-endian value and always transmits
> the least significant byte first, does this logic cause big-endian hosts to
> transmit the bytes backwards instead of most significant byte first?
>

This issue was already mentioned in Sashiko's feedback for [1/4] and as
I said, I will take a look a it and send a new patch if it applies.

> [Severity: High]
> This isn't a bug introduced by this patch, but in cc2_read_reg() and
> cc2_data_fetch(), byte array pointers at odd offsets such as &buf[1] or stack
> arrays are directly cast to __be16 * and passed to be16_to_cpup().
>
> Can these unaligned dereferences trigger kernel panics on CPU architectures
> with strict memory alignment requirements? Would it be safer to use
> get_unaligned_be16() to read these values?

Same here.

Best regards,
Javier