Re: [PATCH v5 07/11] HID: spi-hid: add ACPI support for HID over SPI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] spi-hid: data race on bus_error_count in spi_hid_error_handler
--

commit a262e333d16eb58c8aa593f81c7497790c2b01b7
Author: Angela Czubak <acz@semihalf.com>

HID: spi-hid: add ACPI support for HID over SPI
    
Detect SPI HID devices described in ACPI.

> diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> index 83678e6835818..f4e9b4e1294ed 100644
> --- a/drivers/hid/spi-hid/spi-hid-core.c
> +++ b/drivers/hid/spi-hid/spi-hid-core.c

[ ... ]

> @@ -327,8 +303,21 @@ static void spi_hid_error_handler(struct spi_hid *shid)
>  
>  	shid->power_state = HIDSPI_ON;
>  
> -	shid->ops->deassert_reset(shid->ops);
> +	error = shid->ops->deassert_reset(shid->ops);
> +	if (error) {
> +		dev_err(dev, "failed to deassert reset: %d\n", error);
> +		shid->bus_error_count++;
> +		shid->bus_last_error = error;
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Does this introduce a data race with other updaters of bus_error_count?

Userspace sending an output report via __spi_hid_send_output_report() checks
SPI_HID_READY locklessly. If SPI_HID_READY is read as 1 just before
spi_hid_error_handler() clears it, the output report path proceeds to acquire
io_lock and invoke spi_hid_output(), which updates bus_error_count on error
(under io_lock).

Because spi_hid_error_handler() updates bus_error_count here without holding
io_lock, both threads can execute shid->bus_error_count++ concurrently, which
can lead to lost increments or torn reads via sysfs.

> +		/*
> +		 * Without a successful reset the device will not send a reset
> +		 * response, so no IRQ would trigger another recovery attempt.
> +		 * Retry from here; reset_attempts bounds the number of tries.
> +		 */
> +		set_bit(SPI_HID_ERROR, &shid->flags);
> +		schedule_work(&shid->reset_work);
> +	}
>  
> +	/* Balance disable_irq() above; a retry disables it again. */
>  	enable_irq(shid->spi->irq);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-send-upstream-v5-0-384af01da3ee@chromium.org?part=7