linux-kernel - Re: pcm|dmaengine|imx-sdma race condition on i.MX6

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: <7a16b532-6edd-83b3-6a57-c0b471b9401a@metafoo.de>
Date:   Wed, 19 Aug 2020 13:16:43 +0200
From:   Lars-Peter Clausen <lars@...afoo.de>
To:     Benjamin Bara - SKIDATA <Benjamin.Bara@...data.com>,
        Robin Gong <yibin.gong@....com>
Cc:     "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "timur@...nel.org" <timur@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "nicoleotsuka@...il.com" <nicoleotsuka@...il.com>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        dl-linux-imx <linux-imx@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "dan.j.williams@...el.com" <dan.j.williams@...el.com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Richard Leitner - SKIDATA <Richard.Leitner@...data.com>
Subject: Re: pcm|dmaengine|imx-sdma race condition on i.MX6

On 8/19/20 1:08 PM, Lars-Peter Clausen wrote:
> On 8/17/20 9:28 AM, Benjamin Bara - SKIDATA wrote:
>> We think this is not an i.MX6-specific problem, but a problem of the 
>> DMAengine usage from the PCM.
>> In case of a XRUN, the DMA channel is never closed but first a 
>> SNDRV_PCM_TRIGGER_STOP next a
>> SNDRV_PCM_TRIGGER_START is triggered.
>> The SNDRV_PCM_TRIGGER_STOP simply executes a 
>> dmaengine_terminate_async() [1]
>> but does not await the termination by calling dmaengine_synchronize(),
>> which is required as stated by the docu [2].
>> Anyways, we are not able to fix it in the pcm_dmaengine layer either 
>> at the end of
>> SNDRV_PCM_TRIGGER_STOP (called from the DMA on complete interrupt 
>> handler)
>> or at the beginning of SNDRV_PCM_TRIGGER_START (called from a PCM 
>> ioctl),
>> since the dmaengine_synchronize() requires a non-atomic context.
>
> I think this might be an sdma specific problem after all. 
> dmaengine_terminate_async() will issue a request to stop the DMA. But 
> it is still safe to issue the next transfer, even without calling 
> dmaengine_synchronize(). The DMA should start the new transfer at its 
> earliest convenience in that case.
>
> dmaegine_synchronize() is so that the consumer has a guarantee that 
> the DMA is finished using the resources (e.g. the memory buffers) 
> associated with the DMA transfer so it can safely free them. 

You can think of dmaengine_terminate_async() and 
dmaengine_issue_pending() as adding operations to a command queue. The 
DMA is responsible that the operations are executed in the same order 
that they were added to the queue and to make sure that their execution 
does not conflict.

dmaegine_synchronize() is for external consumers to wait until all 
operations in the command queue have been completed.