Re: [RFC PATCH v2 0/5] ASoC: soc-pcm: fix trigger race conditions with shared BE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>> Using snd_pcm_stream_lock_irqsave(be_substream, flags); will prevent
>> multiple triggers indeed, but the state management is handled by
>> dpcm_lock, so I think we have to use dpcm_lock/mutex in all BE transitions.
>>
>> if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
>>     (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
>>  			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
>>
>> if ((be->dpcm[stream].state != SND_SOC_DPCM_STATE_PREPARE) &&
>>     (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) &&
>>  			    (be->dpcm[stream].state != SND_SOC_DPCM_STATE_PAUSED))
> 
> The stream lock can be put around those appropriate places, I suppose?

I doubled checked the code a bit more, and all functions using
be->dpcm[stream].state and be->dpcm[stream].users are protected by the
card->mutex.

The exceptions are dpcm_be_dai_trigger() and dpcm_show_state() so we
probably don't need to worry too much about these fields.

I am more nervous about that the dpcm_lock was supposed to protect. It
was added in "ASoC: dpcm: prevent snd_soc_dpcm use after free" to solve
a race condition, according to the commit log between

void dpcm_be_disconnect(
    	...
    	list_del(&dpcm->list_be);
    	list_del(&dpcm->list_fe);
    	kfree(dpcm);
    	...

and
    	for_each_dpcm_fe()
    	for_each_dpcm_be*()

That would suggest that every one of those loops should be protected,
but that's not the case at all. In some cases the spinlock is taken
*inside* of the loops.

I think this is what explains the problem reported by Gyeongtaek Lee in

https://lore.kernel.org/alsa-devel/002f01d7b4f5$c030f4a0$4092dde0$@samsung.com/

the for_each_dpcm_be() loop in dpcm_be_dai_trigger() is NOT protected.

But if we add a spin-lock in there, the atomicity remains a problem.

I think the only solution is to follow the example of the PCM case,
where the type of lock depends on the FE types, with the assumption that
there are no mixed atomic/non-atomic FE configurations.



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux