linux-kernel - Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

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]

Message-ID: <1429291b-77c5-41aa-8dee-8858eba6d138@onway.ch>
Date:   Mon, 24 Feb 2020 13:24:04 +0000
From:   Andreas Tobler <andreas.tobler@...ay.ch>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Robin Gong <yibin.gong@....com>, Vinod Koul <vkoul@...nel.org>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 5.4 160/344] dmaengine: imx-sdma: Fix memory leak

Hi all,

On 21.02.20 08:39, Greg Kroah-Hartman wrote:
> From: Sascha Hauer <s.hauer@...gutronix.de>
> 
> [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ]
> 
> The current descriptor is not on any list of the virtual DMA channel.
> Once sdma_terminate_all() is called when a descriptor is currently
> in flight then this one is forgotten to be freed. We have to call
> vchan_terminate_vdesc() on this descriptor to re-add it to the lists.
> Now that we also free the currently running descriptor we can (and
> actually have to) remove the current descriptor from its list also
> for the cyclic case.
> 
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> Reviewed-by: Robin Gong <yibin.gong@....com>
> Tested-by: Robin Gong <yibin.gong@....com>
> Link: https://lore.kernel.org/r/20191216105328.15198-10-s.hauer@pengutronix.de
> Signed-off-by: Vinod Koul <vkoul@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>   drivers/dma/imx-sdma.c | 19 +++++++++++--------
>   1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index c27e206a764c3..66f1b2ac5cde4 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac)
>   		return;
>   	}
>   	sdmac->desc = desc = to_sdma_desc(&vd->tx);
> -	/*
> -	 * Do not delete the node in desc_issued list in cyclic mode, otherwise
> -	 * the desc allocated will never be freed in vchan_dma_desc_free_list
> -	 */
> -	if (!(sdmac->flags & IMX_DMA_SG_LOOP))
> -		list_del(&vd->node);
> +
> +	list_del(&vd->node);
>   
>   	sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
>   	sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
> @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work)
>   
>   	spin_lock_irqsave(&sdmac->vc.lock, flags);
>   	vchan_get_all_descriptors(&sdmac->vc, &head);
> -	sdmac->desc = NULL;
>   	spin_unlock_irqrestore(&sdmac->vc.lock, flags);
>   	vchan_dma_desc_free_list(&sdmac->vc, &head);
>   	sdmac->context_loaded = false;
> @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work)
>   static int sdma_disable_channel_async(struct dma_chan *chan)
>   {
>   	struct sdma_channel *sdmac = to_sdma_chan(chan);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdmac->vc.lock, flags);
>   
>   	sdma_disable_channel(chan);
>   
> -	if (sdmac->desc)
> +	if (sdmac->desc) {
> +		vchan_terminate_vdesc(&sdmac->desc->vd);
> +		sdmac->desc = NULL;
>   		schedule_work(&sdmac->terminate_worker);
> +	}
> +
> +	spin_unlock_irqrestore(&sdmac->vc.lock, flags);
>   
>   	return 0;
>   }
> 

This patch breaks our imx6 board with the attached trace.  Reverting the 
patch makes it boot again.
I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c 
from 5.6-rc3 showed me some details which might have to be backported as 
well to make this patch work.
I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow 
successful. I still have one trace but the board boots now.

Any insights from the experts?
TIA,
Andreas






View attachment "trace_uart_sdma.txt" of type "text/plain" (2083 bytes)