netdev - RE: [PATCH net v2] dpaa2-ptp: fix double free of the ptp

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 Dec 2019 02:24:13 +0000
From:   "Y.b. Lu" <yangbo.lu@....com>
To:     Ioana Ciornei <ioana.ciornei@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net v2] dpaa2-ptp: fix double free of the ptp_qoriq IRQ

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

[Y.b. Lu] Reviewed-by: Yangbo Lu <yangbo.lu@....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@....com>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@....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