Re: [PATCH] dt-bindings: iio: adc: Add schema for AD7816/7/8 digital temperature sensor

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing concurrency protection during sysfs accesses.
- [High] Stack-allocated buffers are passed directly to `spi_read()` and `spi_write()`, violating DMA mapping rules.
- [High] Unbounded polling loop for the BUSY pin can cause a hard CPU lockup.
--

commit cb7e57cc9cdde2668abda84c0f0d3e60ebb9a962
Author: Taha Narimani <tahanarimani3443@gmail.com>

dt-bindings: iio: adc: Add schema for AD7816/7/8 digital temperature sensor

This commit adds a new YAML device tree binding schema for the Analog
Devices AD7816, AD7817, and AD7818 digital temperature sensors. It
documents the compatible strings, SPI configuration, interrupts, and
required GPIO pins.

> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> new file mode 100644
> index 0000000000000..74008c14d40c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7816.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7816.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7816
> +      - adi,ad7817
> +      - adi,ad7818

[Severity: High]
This isn't a bug introduced by this patch, but is there a missing lock to
protect concurrent sysfs accesses?

Looking at ad7816_show_value() in drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_show_value() {
    ...
    ret = ad7816_spi_read(chip, &data);
    ...
}

Since this is called for world-readable sysfs attributes, can multiple
userspace processes trigger this concurrently?

If so, could concurrent execution of ad7816_spi_read() interleave GPIO pin
toggling and SPI transfers, corrupting the hardware state?


[Severity: High]
This isn't a bug introduced by this patch, but does this code pass
stack-allocated buffers to SPI transfers?

Looking at drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_spi_read() {
    ...
    ret = spi_read(spi_dev, &buf, sizeof(*data));
    ...
}

And also in ad7816_spi_write():

drivers/staging/iio/adc/ad7816.c:ad7816_spi_write() {
    ...
    ret = spi_write(spi_dev, &data, sizeof(data));
    ...
}

Since the SPI subsystem often maps these buffers for DMA, will passing
variables from the stack cause mapping failures or crashes on architectures
with virtually mapped kernel stacks?


[Severity: High]
This isn't a bug introduced by this patch, but can this unbounded polling
loop cause a CPU lockup?

Looking at ad7816_spi_read() in drivers/staging/iio/adc/ad7816.c:

drivers/staging/iio/adc/ad7816.c:ad7816_spi_read() {
    ...
    while (gpiod_get_value(chip->busy_pin))
        cpu_relax();
    ...
}

If the external device malfunctions or the GPIO line gets permanently stuck
high, will this loop block indefinitely and cause a hard lockup?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530135302.54688-1-tahanarimani3443@gmail.com?part=1