[v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

[PATCH v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

Posted by Chen Ni 8 months, 4 weeks ago

Fix a race where a pending interrupt could be received and the handler
called before the handler's data has been setup, by converting to
irq_set_chained_handler_and_data().

Fixes: d58c67d1d851 ("iio: adc: stm32-adc: add support for STM32MP1")
Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
---
Changelog:

v1 -> v2:

1. Add Fixes tag.
---
 drivers/iio/adc/stm32-adc-core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-core.c
index bd3458965bff..21c04a98b3b6 100644
--- a/drivers/iio/adc/stm32-adc-core.c
+++ b/drivers/iio/adc/stm32-adc-core.c
@@ -430,10 +430,9 @@ static int stm32_adc_irq_probe(struct platform_device *pdev,
 		return -ENOMEM;
 	}
 
-	for (i = 0; i < priv->cfg->num_irqs; i++) {
-		irq_set_chained_handler(priv->irq[i], stm32_adc_irq_handler);
-		irq_set_handler_data(priv->irq[i], priv);
-	}
+	for (i = 0; i < priv->cfg->num_irqs; i++)
+		irq_set_chained_handler_and_data(priv->irq[i],
+						 stm32_adc_irq_handler, priv);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

Posted by Nuno Sá 8 months, 4 weeks ago

On Thu, 2025-05-15 at 16:31 +0800, Chen Ni wrote:
> Fix a race where a pending interrupt could be received and the handler
> called before the handler's data has been setup, by converting to
> irq_set_chained_handler_and_data().
> 
> Fixes: d58c67d1d851 ("iio: adc: stm32-adc: add support for STM32MP1")
> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

> Changelog:
> 
> v1 -> v2:
> 
> 1. Add Fixes tag.
> ---
>  drivers/iio/adc/stm32-adc-core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-
> core.c
> index bd3458965bff..21c04a98b3b6 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -430,10 +430,9 @@ static int stm32_adc_irq_probe(struct platform_device
> *pdev,
>  		return -ENOMEM;
>  	}
>  
> -	for (i = 0; i < priv->cfg->num_irqs; i++) {
> -		irq_set_chained_handler(priv->irq[i], stm32_adc_irq_handler);
> -		irq_set_handler_data(priv->irq[i], priv);
> -	}
> +	for (i = 0; i < priv->cfg->num_irqs; i++)
> +		irq_set_chained_handler_and_data(priv->irq[i],
> +						 stm32_adc_irq_handler,
> priv);
>  
>  	return 0;
>  }

Re: [PATCH v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

Posted by Jonathan Cameron 8 months, 2 weeks ago

On Thu, 15 May 2025 11:26:56 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:

> On Thu, 2025-05-15 at 16:31 +0800, Chen Ni wrote:
> > Fix a race where a pending interrupt could be received and the handler
> > called before the handler's data has been setup, by converting to
> > irq_set_chained_handler_and_data().
> > 
> > Fixes: d58c67d1d851 ("iio: adc: stm32-adc: add support for STM32MP1")
> > Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
> > ---  
> 
> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
Looks good to me and I've queued it up for after rc1.  If any
ST folk have time to take a look that would be great.

Jonathan

> 
> > Changelog:
> > 
> > v1 -> v2:
> > 
> > 1. Add Fixes tag.
> > ---
> >  drivers/iio/adc/stm32-adc-core.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-
> > core.c
> > index bd3458965bff..21c04a98b3b6 100644
> > --- a/drivers/iio/adc/stm32-adc-core.c
> > +++ b/drivers/iio/adc/stm32-adc-core.c
> > @@ -430,10 +430,9 @@ static int stm32_adc_irq_probe(struct platform_device
> > *pdev,
> >  		return -ENOMEM;
> >  	}
> >  
> > -	for (i = 0; i < priv->cfg->num_irqs; i++) {
> > -		irq_set_chained_handler(priv->irq[i], stm32_adc_irq_handler);
> > -		irq_set_handler_data(priv->irq[i], priv);
> > -	}
> > +	for (i = 0; i < priv->cfg->num_irqs; i++)
> > +		irq_set_chained_handler_and_data(priv->irq[i],
> > +						 stm32_adc_irq_handler,
> > priv);
> >  
> >  	return 0;
> >  }

Re: [PATCH v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

Posted by Fabrice Gasnier 8 months, 2 weeks ago

On 5/25/25 13:07, Jonathan Cameron wrote:
> On Thu, 15 May 2025 11:26:56 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
> 
>> On Thu, 2025-05-15 at 16:31 +0800, Chen Ni wrote:
>>> Fix a race where a pending interrupt could be received and the handler
>>> called before the handler's data has been setup, by converting to
>>> irq_set_chained_handler_and_data().
>>>
>>> Fixes: d58c67d1d851 ("iio: adc: stm32-adc: add support for STM32MP1")
>>> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
>>> ---  
>>
>> Reviewed-by: Nuno Sá <nuno.sa@analog.com>
> Looks good to me and I've queued it up for after rc1.  If any
> ST folk have time to take a look that would be great.

Hi Jonathan,

One minor comment at my end, not sure if that changes a lot...
This could be a fix for the older commit:
1add69880240 ("iio: adc: Add support for STM32 ADC core")

Apart from that, you can add my:
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

BR,
Fabrice

> 
> Jonathan
> 
>>
>>> Changelog:
>>>
>>> v1 -> v2:
>>>
>>> 1. Add Fixes tag.
>>> ---
>>>  drivers/iio/adc/stm32-adc-core.c | 7 +++----
>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-
>>> core.c
>>> index bd3458965bff..21c04a98b3b6 100644
>>> --- a/drivers/iio/adc/stm32-adc-core.c
>>> +++ b/drivers/iio/adc/stm32-adc-core.c
>>> @@ -430,10 +430,9 @@ static int stm32_adc_irq_probe(struct platform_device
>>> *pdev,
>>>  		return -ENOMEM;
>>>  	}
>>>  
>>> -	for (i = 0; i < priv->cfg->num_irqs; i++) {
>>> -		irq_set_chained_handler(priv->irq[i], stm32_adc_irq_handler);
>>> -		irq_set_handler_data(priv->irq[i], priv);
>>> -	}
>>> +	for (i = 0; i < priv->cfg->num_irqs; i++)
>>> +		irq_set_chained_handler_and_data(priv->irq[i],
>>> +						 stm32_adc_irq_handler,
>>> priv);
>>>  
>>>  	return 0;
>>>  }  
>

Re: [PATCH v2] iio: adc: stm32-adc: Fix race in installing chained IRQ handler

Posted by Jonathan Cameron 8 months, 1 week ago

On Mon, 26 May 2025 17:48:31 +0200
Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:

> On 5/25/25 13:07, Jonathan Cameron wrote:
> > On Thu, 15 May 2025 11:26:56 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> >   
> >> On Thu, 2025-05-15 at 16:31 +0800, Chen Ni wrote:  
> >>> Fix a race where a pending interrupt could be received and the handler
> >>> called before the handler's data has been setup, by converting to
> >>> irq_set_chained_handler_and_data().
> >>>
> >>> Fixes: d58c67d1d851 ("iio: adc: stm32-adc: add support for STM32MP1")
> >>> Signed-off-by: Chen Ni <nichen@iscas.ac.cn>
> >>> ---    
> >>
> >> Reviewed-by: Nuno Sá <nuno.sa@analog.com>  
> > Looks good to me and I've queued it up for after rc1.  If any
> > ST folk have time to take a look that would be great.  
> 
> Hi Jonathan,
> 
> One minor comment at my end, not sure if that changes a lot...
> This could be a fix for the older commit:
> 1add69880240 ("iio: adc: Add support for STM32 ADC core")

Agreed. I've tweaked the message.  It won't apply cleanly all that way
but at least we flagged it for anyone who cares to backport past the code
movement in the Fixes tag Chen used.

> 
> Apart from that, you can add my:
> Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Added.
> 
> BR,
> Fabrice
> 
> > 
> > Jonathan
> >   
> >>  
> >>> Changelog:
> >>>
> >>> v1 -> v2:
> >>>
> >>> 1. Add Fixes tag.
> >>> ---
> >>>  drivers/iio/adc/stm32-adc-core.c | 7 +++----
> >>>  1 file changed, 3 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/stm32-adc-core.c b/drivers/iio/adc/stm32-adc-
> >>> core.c
> >>> index bd3458965bff..21c04a98b3b6 100644
> >>> --- a/drivers/iio/adc/stm32-adc-core.c
> >>> +++ b/drivers/iio/adc/stm32-adc-core.c
> >>> @@ -430,10 +430,9 @@ static int stm32_adc_irq_probe(struct platform_device
> >>> *pdev,
> >>>  		return -ENOMEM;
> >>>  	}
> >>>  
> >>> -	for (i = 0; i < priv->cfg->num_irqs; i++) {
> >>> -		irq_set_chained_handler(priv->irq[i], stm32_adc_irq_handler);
> >>> -		irq_set_handler_data(priv->irq[i], priv);
> >>> -	}
> >>> +	for (i = 0; i < priv->cfg->num_irqs; i++)
> >>> +		irq_set_chained_handler_and_data(priv->irq[i],
> >>> +						 stm32_adc_irq_handler,
> >>> priv);
> >>>  
> >>>  	return 0;
> >>>  }    
> >   
>