Re: [PATCH v1 1/5] iio: pressure: bmp280: Tolerate IRQ before registering

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 17 Mar 2020 12:18:09 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> With DEBUG_SHIRQ enabled we have a kernel crash
> 
> [  116.482696] BUG: kernel NULL pointer dereference, address: 0000000000000000
> 
> ...
> 
> [  116.606571] Call Trace:
> [  116.609023]  <IRQ>
> [  116.611047]  complete+0x34/0x50
> [  116.614206]  bmp085_eoc_irq+0x9/0x10 [bmp280]
> 
> because DEBUG_SHIRQ mechanism fires an IRQ before registration and drivers
> ought to be able to handle an interrupt happening before request_irq() returns.
> 
> Fixes: aae953949651 ("iio: pressure: bmp280: add support for BMP085 EOC interrupt")
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Hmm. Nasty corner case but fair enough to fix it up.
I guess we should audit other drivers for similar paths...

> ---
>  drivers/iio/pressure/bmp280-core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index 29c209cc1108..5e229b95308e 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -712,8 +712,7 @@ static int bmp180_measure(struct bmp280_data *data, u8 ctrl_meas)
>  	unsigned int delay_us;
>  	unsigned int ctrl;
>  
> -	if (data->use_eoc)
> -		init_completion(&data->done);
> +	reinit_completion(&data->done);

reinit on the completion when we don't even have an interrupt
(hence data->use_eoc == false) seems excessive.  Why did
you drop the conditional?

Jonathan



>  
>  	ret = regmap_write(data->regmap, BMP280_REG_CTRL_MEAS, ctrl_meas);
>  	if (ret)
> @@ -969,6 +968,9 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
>  			"trying to enforce it\n");
>  		irq_trig = IRQF_TRIGGER_RISING;
>  	}
> +
> +	init_completion(&data->done);
> +
>  	ret = devm_request_threaded_irq(dev,
>  			irq,
>  			bmp085_eoc_irq,