linux-kernel - Re: [PATCH 4/4] ALSA: pcm_dmaengine: Properly synchronize DMA on shutdown

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: <56262810.3040201@metafoo.de>
Date:	Tue, 20 Oct 2015 13:40:00 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Takashi Iwai <tiwai@...e.de>
CC:	Vinod Koul <vinod.koul@...el.com>, alsa-devel@...a-project.org,
	Russell King <rmk+kernel@....linux.org.uk>,
	Shengjiu Wang <shengjiu.wang@...escale.com>,
	Laurent Pinchart <renesas@...asonboard.com>,
	Dan Williams <dan.j.williams@...el.com>,
	Jonah Petri <jpetri@...tope.com>,
	Matt Campbell <mcampbell@...tope.com>,
	Qiao Zhou <zhouqiao@...vell.com>,
	Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
	dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] ALSA: pcm_dmaengine: Properly synchronize DMA on
 shutdown

On 10/20/2015 01:17 PM, Takashi Iwai wrote:
> On Tue, 20 Oct 2015 11:46:31 +0200,
> Lars-Peter Clausen wrote:
>>
>> Use the new dmaengine_synchronize() function to make sure that all complete
>> callbacks have finished running before the runtime data, which is accessed
>> in the completed callback, is freed.
>>
>> This fixes a long standing use-after-free race condition that has been
>> observed on some systems.
> 
> What if a substream is restarted immediately after the stop?
> 

What can happen is that you get a complete callback and the associated
snd_pcm_period_elapsed() too early, before the period has actually elapsed,
but I don't think that this is a problem if the DMA driver properly
implements residue reporting.

This fails if we rely on period counting, but that is broken anyway and
already prone to other race conditions.

I've tested this series with xrun injection and some modifications to the
DMA driver to always trigger the race condition when the stream is stopped.
And I've not seen any issues after the transfer re-started. (There is a
dead-lock condition though but that does not seem to be related to this series)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/