From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:44176 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750975AbeBQOSl (ORCPT ); Sat, 17 Feb 2018 09:18:41 -0500 Date: Sat, 17 Feb 2018 14:18:36 +0000 From: Jonathan Cameron To: Lars-Peter Clausen Cc: Hartmut Knaack , Peter Meerwald-Stadler , linux-iio@vger.kernel.org Subject: Re: [PATCH] iio: adis_lib: Initialize trigger before requesting interrupt Message-ID: <20180217141836.6303ab3a@archlinux> In-Reply-To: <20180214144300.27550-1-lars@metafoo.de> References: <20180214144300.27550-1-lars@metafoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On Wed, 14 Feb 2018 15:43:00 +0100 Lars-Peter Clausen 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 : [] lr : [] 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) > [] (iio_trigger_notify_done) from [] (__handle_irq_event_percpu+0x88/0x118) > [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x1c/0x58) > [] (handle_irq_event_percpu) from [] (handle_irq_event+0x38/0x5c) > [] (handle_irq_event) from [] (handle_level_irq+0xa4/0x130) > [] (handle_level_irq) from [] (generic_handle_irq+0x24/0x34) > [] (generic_handle_irq) from [] (zynq_gpio_irqhandler+0xb8/0x13c) > [] (zynq_gpio_irqhandler) from [] (generic_handle_irq+0x24/0x34) > [] (generic_handle_irq) from [] (__handle_domain_irq+0x5c/0xb4) > [] (__handle_domain_irq) from [] (gic_handle_irq+0x48/0x8c) > [] (gic_handle_irq) from [] (__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 > Signed-off-by: Lars-Peter Clausen 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);