On Wed, 14 Feb 2018 15:43:00 +0100
Lars-Peter Clausen <lars@xxxxxxxxxx> wrote:
> The adis_probe_trigger() creates a new IIO trigger and requests an
> interrupt associated with the trigger. The interrupt uses the generic
> iio_trigger_generic_data_rdy_poll() function as its interrupt handler.
>
> Currently the driver initializes some fields of the trigger structure after
> the interrupt has been requested. But an interrupt can fire as soon as it
> has been requested. This opens up a race condition.
>
> iio_trigger_generic_data_rdy_poll() will access the trigger data structure
> and dereference the ops field. If the ops field is not yet initialized this
> will result in a NULL pointer deref.
>
> It is not expected that the device generates an interrupt at this point, so
> typically this issue did not surface unless e.g. due to a hardware
> misconfiguration (wrong interrupt number, wrong polarity, etc.).
>
> But some newer devices from the ADIS family start to generate periodic
> interrupts in their power-on reset configuration and unfortunately the
> interrupt can not be masked in the device. This makes the race condition
> much more visible and the following crash has been observed occasionally
> when booting a system using the ADIS16460.
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000008
> pgd = c0004000
> [00000008] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.9.0-04126-gf9739f0-dirty #257
> Hardware name: Xilinx Zynq Platform
> task: ef04f640 task.stack: ef050000
> PC is at iio_trigger_notify_done+0x30/0x68
> LR is at iio_trigger_generic_data_rdy_poll+0x18/0x20
> pc : [<c042d868>] lr : [<c042d924>] psr: 60000193
> sp : ef051bb8 ip : 00000000 fp : ef106400
> r10: c081d80a r9 : ef3bfa00 r8 : 00000087
> r7 : ef051bec r6 : 00000000 r5 : ef3bfa00 r4 : ee92ab00
> r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : ee97e400
> Flags: nZCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment none
> Control: 18c5387d Table: 0000404a DAC: 00000051
> Process swapper/0 (pid: 1, stack limit = 0xef050210)
> [<c042d868>] (iio_trigger_notify_done) from [<c0065b10>] (__handle_irq_event_percpu+0x88/0x118)
> [<c0065b10>] (__handle_irq_event_percpu) from [<c0065bbc>] (handle_irq_event_percpu+0x1c/0x58)
> [<c0065bbc>] (handle_irq_event_percpu) from [<c0065c30>] (handle_irq_event+0x38/0x5c)
> [<c0065c30>] (handle_irq_event) from [<c0068e28>] (handle_level_irq+0xa4/0x130)
> [<c0068e28>] (handle_level_irq) from [<c0064e74>] (generic_handle_irq+0x24/0x34)
> [<c0064e74>] (generic_handle_irq) from [<c021ab7c>] (zynq_gpio_irqhandler+0xb8/0x13c)
> [<c021ab7c>] (zynq_gpio_irqhandler) from [<c0064e74>] (generic_handle_irq+0x24/0x34)
> [<c0064e74>] (generic_handle_irq) from [<c0065370>] (__handle_domain_irq+0x5c/0xb4)
> [<c0065370>] (__handle_domain_irq) from [<c000940c>] (gic_handle_irq+0x48/0x8c)
> [<c000940c>] (gic_handle_irq) from [<c0013e8c>] (__irq_svc+0x6c/0xa8)
>
> To fix this make sure that the trigger is fully initialized before
> requesting the interrupt.
>
> Fixes: ccd2b52f4ac6 ("staging:iio: Add common ADIS library")
> Reported-by: Robin Getz <Robin.Getz@xxxxxxxxxx>
> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
Applied to the fixes-togreg-post-rc1 branch of iio.git and marked
for stable.
Thanks,
Jonathan
> ---
> drivers/iio/imu/adis_trigger.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/imu/adis_trigger.c b/drivers/iio/imu/adis_trigger.c
> index 0dd5a381be64..457372f36791 100644
> --- a/drivers/iio/imu/adis_trigger.c
> +++ b/drivers/iio/imu/adis_trigger.c
> @@ -46,6 +46,10 @@ int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> if (adis->trig == NULL)
> return -ENOMEM;
>
> + adis->trig->dev.parent = &adis->spi->dev;
> + adis->trig->ops = &adis_trigger_ops;
> + iio_trigger_set_drvdata(adis->trig, adis);
> +
> ret = request_irq(adis->spi->irq,
> &iio_trigger_generic_data_rdy_poll,
> IRQF_TRIGGER_RISING,
> @@ -54,9 +58,6 @@ int adis_probe_trigger(struct adis *adis, struct iio_dev *indio_dev)
> if (ret)
> goto error_free_trig;
>
> - adis->trig->dev.parent = &adis->spi->dev;
> - adis->trig->ops = &adis_trigger_ops;
> - iio_trigger_set_drvdata(adis->trig, adis);
> ret = iio_trigger_register(adis->trig);
>
> indio_dev->trig = iio_trigger_get(adis->trig);
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
