| Message ID | 1576510350-28660-1-git-send-email-ioana.ciornei@nxp.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net,v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ | expand |
> -----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
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.
> -----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>
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.
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.
> -----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
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:
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(-)