linux-kernel - Re: [PATCHv2] iio: Fix error handling in iio_trigger_attach_poll

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bcc2f693-5882-7ed2-5d8a-ea5bf3d1e678@kernel.org>
Date:	Wed, 4 May 2016 10:13:12 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Crestez Dan Leonard <leonard.crestez@...el.com>,
	linux-iio@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Daniel Baluta <daniel.baluta@...el.com>
Subject: Re: [PATCHv2] iio: Fix error handling in iio_trigger_attach_poll_func

On 03/05/16 13:27, Crestez Dan Leonard wrote:
> When attaching a pollfunc iio_trigger_attach_poll_func will allocate a
> virtual irq and call the driver's set_trigger_state function. Fix error
> handling to undo previous steps if any fails.
> 
> In particular this fixes handling errors from a driver's
> set_trigger_state function. When using triggered buffers a failure to
> enable the trigger used to make the buffer unusable.
> 
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@...el.com>
Thought hard about this but in the end marked it for stable as can be caused
by hardware issues and perhaps some configuration path I've missed in an existing
driver.

Applied to the fixes-togreg branch of iio.git (I was going to do that whatever!)

Thanks for fixing this mess up,

Jonathan
> ---
> 
> Previous version was sent as a RFC. This version also properly handles errors
> from iio_trigger_get_irq and request_threaded_irq.
> 
> Calling request_threaded_irq with a negative/invalid irq parameter might work
> and return an error but let's not rely on that.
> 
>  drivers/iio/industrialio-trigger.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-trigger.c b/drivers/iio/industrialio-trigger.c
> index ae2806a..0c52dfe 100644
> --- a/drivers/iio/industrialio-trigger.c
> +++ b/drivers/iio/industrialio-trigger.c
> @@ -210,22 +210,35 @@ static int iio_trigger_attach_poll_func(struct iio_trigger *trig,
>  
>  	/* Prevent the module from being removed whilst attached to a trigger */
>  	__module_get(pf->indio_dev->info->driver_module);
> +
> +	/* Get irq number */
>  	pf->irq = iio_trigger_get_irq(trig);
> +	if (pf->irq < 0)
> +		goto out_put_module;
> +
> +	/* Request irq */
>  	ret = request_threaded_irq(pf->irq, pf->h, pf->thread,
>  				   pf->type, pf->name,
>  				   pf);
> -	if (ret < 0) {
> -		module_put(pf->indio_dev->info->driver_module);
> -		return ret;
> -	}
> +	if (ret < 0)
> +		goto out_put_irq;
>  
> +	/* Enable trigger in driver */
>  	if (trig->ops && trig->ops->set_trigger_state && notinuse) {
>  		ret = trig->ops->set_trigger_state(trig, true);
>  		if (ret < 0)
> -			module_put(pf->indio_dev->info->driver_module);
> +			goto out_free_irq;
>  	}
>  
>  	return ret;
> +
> +out_free_irq:
> +	free_irq(pf->irq, pf);
> +out_put_irq:
> +	iio_trigger_put_irq(trig, pf->irq);
> +out_put_module:
> +	module_put(pf->indio_dev->info->driver_module);
> +	return ret;
>  }
>  
>  static int iio_trigger_detach_poll_func(struct iio_trigger *trig,
>