linux-kernel - Re: [PATCH] mmc: owl-mmc: Fix dma_chan refcnt leak in owl_mmc

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

Message-ID: <282da79a-eda7-f534-a6ed-8ac38fcc2c8b@web.de>
Date:   Sat, 25 Apr 2020 21:30:22 +0200
From:   Markus Elfring <Markus.Elfring@....de>
To:     Xiyu Yang <xiyuyang19@...an.edu.cn>,
        Xin Tan <tanxin.ctf@...il.com>, linux-mmc@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Cc:     linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        Andreas Färber <afaerber@...e.de>,
        Kangjie Lu <kjlu@....edu>,
        Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        Yuan Zhang <yuanxzhang@...an.edu.cn>
Subject: Re: [PATCH] mmc: owl-mmc: Fix dma_chan refcnt leak in owl_mmc_probe()

> Fix this issue by jumping to "err_put_dma" label when those error
> scenarios occur.

I suggest to reconsider your jump target selection.

…
> +++ b/drivers/mmc/host/owl-mmc.c
…
> @@ -643,19 +643,22 @@ static int owl_mmc_probe(struct platform_device *pdev)
>  	return 0;
>
> +err_put_dma:
> +	if (owl_host->dma)
> +		dma_release_channel(owl_host->dma);

I interpret the source code in the way that you would like to call
this function for the desired exception handling only after a call
of the function “dma_request_chan” succeeded.
Thus I would expect that the passed pointer will usually be still valid.
(Can the proposed null pointer check be omitted then?)

How do you think about the following change possibility?

+err_release_channel:
+	dma_release_channel(owl_host->dma);

Would you like to add the tag “Fixes” to the change description?

Regards,
Markus