All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
@ 2019-12-16 15:32 Ioana Ciornei
  2019-12-17  2:24 ` Y.b. Lu
  2019-12-17 22:11 ` David Miller
  0 siblings, 2 replies; 7+ messages in thread
From: Ioana Ciornei @ 2019-12-16 15:32 UTC (permalink / raw)
  To: davem, netdev; +Cc: Ioana Ciornei, Yangbo Lu

Upon reusing the ptp_qoriq driver, the ptp_qoriq_free() function was
used on the remove path to free any allocated resources.
The ptp_qoriq IRQ is among these resources that are freed in
ptp_qoriq_free() even though it is also a managed one (allocated using
devm_request_threaded_irq).

Drop the resource managed version of requesting the IRQ in order to not
trigger a double free of the interrupt as below:

[  226.731005] Trying to free already-free IRQ 126
[  226.735533] WARNING: CPU: 6 PID: 749 at kernel/irq/manage.c:1707
__free_irq+0x9c/0x2b8
[  226.743435] Modules linked in:
[  226.746480] CPU: 6 PID: 749 Comm: bash Tainted: G        W
5.4.0-03629-gfd7102c32b2c-dirty #912
[  226.755857] Hardware name: NXP Layerscape LX2160ARDB (DT)
[  226.761244] pstate: 40000085 (nZcv daIf -PAN -UAO)
[  226.766022] pc : __free_irq+0x9c/0x2b8
[  226.769758] lr : __free_irq+0x9c/0x2b8
[  226.773493] sp : ffff8000125039f0
(...)
[  226.856275] Call trace:
[  226.858710]  __free_irq+0x9c/0x2b8
[  226.862098]  free_irq+0x30/0x70
[  226.865229]  devm_irq_release+0x14/0x20
[  226.869054]  release_nodes+0x1b0/0x220
[  226.872790]  devres_release_all+0x34/0x50
[  226.876790]  device_release_driver_internal+0x100/0x1c0

Fixes: d346c9e86d86 ("dpaa2-ptp: reuse ptp_qoriq driver")
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
Changes in v2:
 - change a goto err_free_mc_irq to err_free_threaded_irq

 drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
index a9503aea527f..6437fe6b9abf 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
@@ -160,10 +160,10 @@ static int dpaa2_ptp_probe(struct fsl_mc_device *mc_dev)
 	irq = mc_dev->irqs[0];
 	ptp_qoriq->irq = irq->msi_desc->irq;
 
-	err = devm_request_threaded_irq(dev, ptp_qoriq->irq, NULL,
-					dpaa2_ptp_irq_handler_thread,
-					IRQF_NO_SUSPEND | IRQF_ONESHOT,
-					dev_name(dev), ptp_qoriq);
+	err = request_threaded_irq(ptp_qoriq->irq, NULL,
+				   dpaa2_ptp_irq_handler_thread,
+				   IRQF_NO_SUSPEND | IRQF_ONESHOT,
+				   dev_name(dev), ptp_qoriq);
 	if (err < 0) {
 		dev_err(dev, "devm_request_threaded_irq(): %d\n", err);
 		goto err_free_mc_irq;
@@ -173,18 +173,20 @@ static int dpaa2_ptp_probe(struct fsl_mc_device *mc_dev)
 				   DPRTC_IRQ_INDEX, 1);
 	if (err < 0) {
 		dev_err(dev, "dprtc_set_irq_enable(): %d\n", err);
-		goto err_free_mc_irq;
+		goto err_free_threaded_irq;
 	}
 
 	err = ptp_qoriq_init(ptp_qoriq, base, &dpaa2_ptp_caps);
 	if (err)
-		goto err_free_mc_irq;
+		goto err_free_threaded_irq;
 
 	dpaa2_phc_index = ptp_qoriq->phc_index;
 	dev_set_drvdata(dev, ptp_qoriq);
 
 	return 0;
 
+err_free_threaded_irq:
+	free_irq(ptp_qoriq->irq, ptp_qoriq);
 err_free_mc_irq:
 	fsl_mc_free_irqs(mc_dev);
 err_unmap:
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* RE: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-16 15:32 [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ Ioana Ciornei
@ 2019-12-17  2:24 ` Y.b. Lu
  2019-12-17  3:12   ` David Miller
  2019-12-17 22:11 ` David Miller
  1 sibling, 1 reply; 7+ messages in thread
From: Y.b. Lu @ 2019-12-17  2:24 UTC (permalink / raw)
  To: Ioana Ciornei, davem, netdev

> -----Original Message-----
> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> Sent: Monday, December 16, 2019 11:33 PM
> To: davem@davemloft.net; netdev@vger.kernel.org
> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; Y.b. Lu <yangbo.lu@nxp.com>
> Subject: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ

[Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>

> 
> Upon reusing the ptp_qoriq driver, the ptp_qoriq_free() function was used on
> the remove path to free any allocated resources.
> The ptp_qoriq IRQ is among these resources that are freed in
> ptp_qoriq_free() even though it is also a managed one (allocated using
> devm_request_threaded_irq).
> 
> Drop the resource managed version of requesting the IRQ in order to not
> trigger a double free of the interrupt as below:
> 
> [  226.731005] Trying to free already-free IRQ 126 [  226.735533] WARNING:
> CPU: 6 PID: 749 at kernel/irq/manage.c:1707
> __free_irq+0x9c/0x2b8
> [  226.743435] Modules linked in:
> [  226.746480] CPU: 6 PID: 749 Comm: bash Tainted: G        W
> 5.4.0-03629-gfd7102c32b2c-dirty #912
> [  226.755857] Hardware name: NXP Layerscape LX2160ARDB (DT)
> [  226.761244] pstate: 40000085 (nZcv daIf -PAN -UAO) [  226.766022] pc :
> __free_irq+0x9c/0x2b8 [  226.769758] lr : __free_irq+0x9c/0x2b8
> [  226.773493] sp : ffff8000125039f0
> (...)
> [  226.856275] Call trace:
> [  226.858710]  __free_irq+0x9c/0x2b8
> [  226.862098]  free_irq+0x30/0x70
> [  226.865229]  devm_irq_release+0x14/0x20 [  226.869054]
> release_nodes+0x1b0/0x220 [  226.872790]  devres_release_all+0x34/0x50
> [  226.876790]  device_release_driver_internal+0x100/0x1c0
> 
> Fixes: d346c9e86d86 ("dpaa2-ptp: reuse ptp_qoriq driver")
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> Changes in v2:
>  - change a goto err_free_mc_irq to err_free_threaded_irq
> 
>  drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
> index a9503aea527f..6437fe6b9abf 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-ptp.c
> @@ -160,10 +160,10 @@ static int dpaa2_ptp_probe(struct fsl_mc_device
> *mc_dev)
>  	irq = mc_dev->irqs[0];
>  	ptp_qoriq->irq = irq->msi_desc->irq;
> 
> -	err = devm_request_threaded_irq(dev, ptp_qoriq->irq, NULL,
> -					dpaa2_ptp_irq_handler_thread,
> -					IRQF_NO_SUSPEND | IRQF_ONESHOT,
> -					dev_name(dev), ptp_qoriq);
> +	err = request_threaded_irq(ptp_qoriq->irq, NULL,
> +				   dpaa2_ptp_irq_handler_thread,
> +				   IRQF_NO_SUSPEND | IRQF_ONESHOT,
> +				   dev_name(dev), ptp_qoriq);
>  	if (err < 0) {
>  		dev_err(dev, "devm_request_threaded_irq(): %d\n", err);
>  		goto err_free_mc_irq;
> @@ -173,18 +173,20 @@ static int dpaa2_ptp_probe(struct fsl_mc_device
> *mc_dev)
>  				   DPRTC_IRQ_INDEX, 1);
>  	if (err < 0) {
>  		dev_err(dev, "dprtc_set_irq_enable(): %d\n", err);
> -		goto err_free_mc_irq;
> +		goto err_free_threaded_irq;
>  	}
> 
>  	err = ptp_qoriq_init(ptp_qoriq, base, &dpaa2_ptp_caps);
>  	if (err)
> -		goto err_free_mc_irq;
> +		goto err_free_threaded_irq;
> 
>  	dpaa2_phc_index = ptp_qoriq->phc_index;
>  	dev_set_drvdata(dev, ptp_qoriq);
> 
>  	return 0;
> 
> +err_free_threaded_irq:
> +	free_irq(ptp_qoriq->irq, ptp_qoriq);
>  err_free_mc_irq:
>  	fsl_mc_free_irqs(mc_dev);
>  err_unmap:
> --
> 1.9.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-17  2:24 ` Y.b. Lu
@ 2019-12-17  3:12   ` David Miller
  2019-12-17  3:25     ` Y.b. Lu
  0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2019-12-17  3:12 UTC (permalink / raw)
  To: yangbo.lu; +Cc: ioana.ciornei, netdev

From: "Y.b. Lu" <yangbo.lu@nxp.com>
Date: Tue, 17 Dec 2019 02:24:13 +0000

>> -----Original Message-----
>> From: Ioana Ciornei <ioana.ciornei@nxp.com>
>> Sent: Monday, December 16, 2019 11:33 PM
>> To: davem@davemloft.net; netdev@vger.kernel.org
>> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; Y.b. Lu <yangbo.lu@nxp.com>
>> Subject: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
> 
> [Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>

Please start your tags on the first column of the line, do not
add these "[Y.b. Lu] " prefixes as it can confuse automated tools
that look for the tags.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-17  3:12   ` David Miller
@ 2019-12-17  3:25     ` Y.b. Lu
  2019-12-17 22:12       ` David Miller
  0 siblings, 1 reply; 7+ messages in thread
From: Y.b. Lu @ 2019-12-17  3:25 UTC (permalink / raw)
  To: David Miller; +Cc: Ioana Ciornei, netdev

> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Tuesday, December 17, 2019 11:12 AM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
> 
> From: "Y.b. Lu" <yangbo.lu@nxp.com>
> Date: Tue, 17 Dec 2019 02:24:13 +0000
> 
> >> -----Original Message-----
> >> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> >> Sent: Monday, December 16, 2019 11:33 PM
> >> To: davem@davemloft.net; netdev@vger.kernel.org
> >> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; Y.b. Lu <yangbo.lu@nxp.com>
> >> Subject: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
> >
> > [Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>
> 
> Please start your tags on the first column of the line, do not
> add these "[Y.b. Lu] " prefixes as it can confuse automated tools
> that look for the tags.

[Y.b. Lu] Sorry, David. I will remember that :)

Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-16 15:32 [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ Ioana Ciornei
  2019-12-17  2:24 ` Y.b. Lu
@ 2019-12-17 22:11 ` David Miller
  1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2019-12-17 22:11 UTC (permalink / raw)
  To: ioana.ciornei; +Cc: netdev, yangbo.lu

From: Ioana Ciornei <ioana.ciornei@nxp.com>
Date: Mon, 16 Dec 2019 17:32:30 +0200

> Upon reusing the ptp_qoriq driver, the ptp_qoriq_free() function was
> used on the remove path to free any allocated resources.
> The ptp_qoriq IRQ is among these resources that are freed in
> ptp_qoriq_free() even though it is also a managed one (allocated using
> devm_request_threaded_irq).
> 
> Drop the resource managed version of requesting the IRQ in order to not
> trigger a double free of the interrupt as below:
 ...
> Fixes: d346c9e86d86 ("dpaa2-ptp: reuse ptp_qoriq driver")
> Cc: Yangbo Lu <yangbo.lu@nxp.com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>

Applied and queued up for v5.3 -stable.

Thanks.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-17  3:25     ` Y.b. Lu
@ 2019-12-17 22:12       ` David Miller
  2019-12-18  2:11         ` Y.b. Lu
  0 siblings, 1 reply; 7+ messages in thread
From: David Miller @ 2019-12-17 22:12 UTC (permalink / raw)
  To: yangbo.lu; +Cc: ioana.ciornei, netdev

From: "Y.b. Lu" <yangbo.lu@nxp.com>
Date: Tue, 17 Dec 2019 03:25:23 +0000

>> -----Original Message-----
>> From: David Miller <davem@davemloft.net>
>> Sent: Tuesday, December 17, 2019 11:12 AM
>> To: Y.b. Lu <yangbo.lu@nxp.com>
>> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; netdev@vger.kernel.org
>> Subject: Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
>> 
>> From: "Y.b. Lu" <yangbo.lu@nxp.com>
>> Date: Tue, 17 Dec 2019 02:24:13 +0000
>> 
>> >> -----Original Message-----
>> >> From: Ioana Ciornei <ioana.ciornei@nxp.com>
>> >> Sent: Monday, December 16, 2019 11:33 PM
>> >> To: davem@davemloft.net; netdev@vger.kernel.org
>> >> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; Y.b. Lu <yangbo.lu@nxp.com>
>> >> Subject: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
>> >
>> > [Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>
>> 
>> Please start your tags on the first column of the line, do not
>> add these "[Y.b. Lu] " prefixes as it can confuse automated tools
>> that look for the tags.
> 
> [Y.b. Lu] Sorry, David. I will remember that :)

How about completely not using these silly prefixes in your replies?

Nobody else does this, and the quoting of the email says clearly what
you are saying in reply and what is the content you are replying to.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
  2019-12-17 22:12       ` David Miller
@ 2019-12-18  2:11         ` Y.b. Lu
  0 siblings, 0 replies; 7+ messages in thread
From: Y.b. Lu @ 2019-12-18  2:11 UTC (permalink / raw)
  To: David Miller; +Cc: Ioana Ciornei, netdev

> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Wednesday, December 18, 2019 6:12 AM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ
> 
> From: "Y.b. Lu" <yangbo.lu@nxp.com>
> Date: Tue, 17 Dec 2019 03:25:23 +0000
> 
> >> -----Original Message-----
> >> From: David Miller <davem@davemloft.net>
> >> Sent: Tuesday, December 17, 2019 11:12 AM
> >> To: Y.b. Lu <yangbo.lu@nxp.com>
> >> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; netdev@vger.kernel.org
> >> Subject: Re: [PATCH net v2] dpaa2-ptp: fix double free of the
> >> ptp_qoriq IRQ
> >>
> >> From: "Y.b. Lu" <yangbo.lu@nxp.com>
> >> Date: Tue, 17 Dec 2019 02:24:13 +0000
> >>
> >> >> -----Original Message-----
> >> >> From: Ioana Ciornei <ioana.ciornei@nxp.com>
> >> >> Sent: Monday, December 16, 2019 11:33 PM
> >> >> To: davem@davemloft.net; netdev@vger.kernel.org
> >> >> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>; Y.b. Lu
> >> >> <yangbo.lu@nxp.com>
> >> >> Subject: [PATCH net v2] dpaa2-ptp: fix double free of the
> >> >> ptp_qoriq IRQ
> >> >
> >> > [Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@nxp.com>
> >>
> >> Please start your tags on the first column of the line, do not add
> >> these "[Y.b. Lu] " prefixes as it can confuse automated tools that
> >> look for the tags.
> >
> > [Y.b. Lu] Sorry, David. I will remember that :)
> 
> How about completely not using these silly prefixes in your replies?
> 
> Nobody else does this, and the quoting of the email says clearly what you are
> saying in reply and what is the content you are replying to.

Sure. This is a habit for company internal emails, but I will drop the prefixes for linux community discussion.
Thanks:)

Best regards,
Yangbo Lu


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-12-18  2:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 15:32 [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ Ioana Ciornei
2019-12-17  2:24 ` Y.b. Lu
2019-12-17  3:12   ` David Miller
2019-12-17  3:25     ` Y.b. Lu
2019-12-17 22:12       ` David Miller
2019-12-18  2:11         ` Y.b. Lu
2019-12-17 22:11 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.