Re: [PATCH 2/2] iio: accel: adxl367: add support for INT2 interrupt pin

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

New issues:
- [High] The driver swallows -EPROBE_DEFER and other negative error codes from fwnode_irq_get_byname(), breaking probe deferral.

Pre-existing issues:
- [High] Out-of-bounds write in adxl367_push_fifo_data due to missing bounds check on fifo_entries.
- [High] Kernel panic due to a modulo-by-zero operation in adxl367_push_fifo_data when fifo_set_size is uninitialized.
--

--- Patch [2]: [PATCH 2/2] iio: accel: adxl367: add support for INT2 interrupt pin ---
commit 726b611b27f821c71f931174d7f0a134dabcdb00
Author: Antoniu Miclaus <[email protected]>

iio: accel: adxl367: add support for INT2 interrupt pin

The ADXL367 provides two independent interrupt output pins, INT1 and
INT2, each with its own event mapping register sharing an identical bit
layout. This commit determines the connected pin from the
interrupt-names device tree property and routes the interrupt mappings
to the matching register, defaulting to INT1 if no interrupt-names are
present.

> diff --git a/drivers/iio/accel/adxl367.c b/drivers/iio/accel/adxl367.c
> index 8c3de11a10a37..229974478d10b 100644
> --- a/drivers/iio/accel/adxl367.c
> +++ b/drivers/iio/accel/adxl367.c
[ ... ]
> @@ -1426,6 +1430,27 @@ static int adxl367_setup(struct adxl367_state *st)
>  	return adxl367_set_measure_en(st, true);
>  }
>  
> +static int adxl367_get_int_line(struct adxl367_state *st, int irq)
> +{
> +	int int_irq;
> +
> +	int_irq = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT1");
> +	if (int_irq > 0) {

[Severity: High]
Could this condition inadvertently swallow negative error codes like
-EPROBE_DEFER? If fwnode_irq_get_byname() returns -EPROBE_DEFER because the
interrupt controller is not yet ready, this check treats it as a missing
property instead of properly deferring the probe. This can cause the driver
to fall back to trying INT2 or returning the default irq. If the default irq
is 0, devm_request_threaded_irq() will subsequently fail with -EINVAL.

> +		st->int_map_reg = ADXL367_REG_INT1_MAP;
> +		return int_irq;
> +	}
> +
> +	int_irq = fwnode_irq_get_byname(dev_fwnode(st->dev), "INT2");
> +	if (int_irq > 0) {

[Severity: High]
Does this check suffer from the same -EPROBE_DEFER issue as above?

> +		st->int_map_reg = ADXL367_REG_INT2_MAP;
> +		return int_irq;
> +	}
> +
> +	/* No interrupt-names: default to INT1 for backwards compatibility. */
> +	st->int_map_reg = ADXL367_REG_INT1_MAP;
> +	return irq;
> +}

[Severity: High]
This is a pre-existing issue, but I noticed a potential out-of-bounds write
in adxl367_push_fifo_data():

drivers/iio/accel/adxl367.c:adxl367_push_fifo_data()
    ret = st->ops->read_fifo(st->context, st->fifo_buf, fifo_entries);

Is it possible to overflow st->fifo_buf here? The fifo_entries value is read
from the hardware as a 10-bit value (up to 1023), but st->fifo_buf is fixed
at 512 elements. If the hardware returns a value greater than 512, this
would cause an out-of-bounds write.

[Severity: High]
This is also a pre-existing issue, but there appears to be a potential
modulo-by-zero panic in the same function:

drivers/iio/accel/adxl367.c:adxl367_push_fifo_data()
    fifo_entries -= fifo_entries % st->fifo_set_size;

Can this cause a kernel panic? If an interrupt triggers before the IIO
buffer is enabled (where st->fifo_set_size is populated), st->fifo_set_size
will be zero. If the ADXL367_STATUS_FIFO_FULL_MASK bit happens to be set,
this division by zero will crash the kernel.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2