linux-kernel - Re: [PATCH 1/2] ASoC: AMD: Fix simultaneous playback and capture on different channel

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

Message-ID: <df52a9e6-457d-754d-ce6e-333752e97ae2@amd.com>
Date:   Mon, 10 Sep 2018 22:51:13 +0530
From:   "Agrawal, Akshu" <Akshu.Agrawal@....com>
To:     Mark Brown <broonie@...nel.org>
Cc:     djkurtz@...omium.org, Alexander.Deucher@....com,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        "Mukunda, Vijendar" <Vijendar.Mukunda@....com>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        Wei Yongjun <weiyongjun1@...wei.com>,
        "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." 
        <alsa-devel@...a-project.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] ASoC: AMD: Fix simultaneous playback and capture on
 different channel



On 9/10/2018 5:08 PM, Mark Brown wrote:
> On Mon, Sep 10, 2018 at 01:36:29PM +0530, Akshu Agrawal wrote:
>> If capture and playback are started on different channel (I2S/BT)
>> there is a possibilty that channel information passed from machine driver
>> is overwritten before the configuration is done in dma driver.
>> Example:
>> 113.597588: cz_max_startup: ---playback sets BT channel
>> 113.597694: cz_dmic1_startup: ---capture sets I2S channel
>> 113.597979: acp_dma_hw_params: ---configures capture for I2S channel
>> 113.598114: acp_dma_hw_params: ---configures playback for I2S channel
>>
>> This is fixed by having lock between startup and prepare. This ensures
>> no other codec startup gets called between a codec's startup(where channel
>> info is set) and hw_params(where channel info is read).
> 
> This isn't viable - the driver will deadlock if the application hits an
> error and never gets to startup, or if the application tries to
> simultaneously configure two channels (ie, do all the prepares and then
> all the parameter configuration and then startup).

We can avoid deadlock by having another mutex_unlock in the shutdown
call of each of codec's ops. Wouldn't in all possible termination
scenarios, it will cleanup and exit via shutdown callback?

Having said that I think there is a better approach to this, is by
having 2 separate instance variable for playback and capture for passing
instance info from machine driver to dma driver. Respective codec in
machine driver will set the capture/playback instance. dma driver on the
basis of substream->stream can read the correct one. No fear of deadlock
in this.

Thanks,
Akshu