Concurrent non-atomic actions on different members of a linked PCM group
currently lock their own buffer first, then the other members. That
acquisition order can differ between callers. Sibling buffer mutexes
also do not close their buffer-access admission counters.
Close buffer access for every member before taking any buffer mutex, and
acquire those mutexes in the stable linked-list order while group
membership is protected. Release every access gate on success and on
failure, including when a member already has an active buffer access.
The group action then uses the buffers already held by its caller.
The PCM stream and group locks are not held across sleeping driver
prepare callbacks. Single-stream non-atomic actions keep their existing
buffer-access locking path.
Fixes: 3c3201f8c7bb ("ALSA: pcm: Fix races among concurrent prepare and
hw_params/hw_free calls")
Fixes: bc55cfd5718c ("ALSA: pcm: Fix potential AB/BA lock with buffer_mutex and
mmap_lock")
Assisted-by: LLM
Signed-off-by: Joonho Lee <[email protected]>
---
sound/core/pcm_native.c | 71 +++++++++++++++++++++++++++++++++++++----------
1 file changed, 56 insertions(+), 15 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -731,6 +731,44 @@
{
mutex_unlock(&runtime->buffer_mutex);
atomic_inc(&runtime->buffer_accessing);
+}
+
+/* Gate all linked buffers before taking their mutexes in group-list order. */
+static int snd_pcm_group_buffer_access_lock(struct snd_pcm_substream
*substream)
+{
+ struct snd_pcm_substream *s, *s1;
+ int depth = 0;
+
+ snd_pcm_group_for_each_entry(s, substream) {
+ if (!atomic_dec_unless_positive(&s->runtime->buffer_accessing))
{
+ snd_pcm_group_for_each_entry(s1, substream) {
+ if (s1 == s)
+ break;
+ atomic_inc(&s1->runtime->buffer_accessing);
+ }
+ return -EBUSY;
+ }
+ }
+
+ snd_pcm_group_for_each_entry(s, substream) {
+ if (depth == 0)
+ mutex_lock(&s->runtime->buffer_mutex);
+ else
+ mutex_lock_nested(&s->runtime->buffer_mutex, depth);
+ depth++;
+ }
+
+ return 0;
+}
+
+static void snd_pcm_group_buffer_access_unlock(struct snd_pcm_substream
*substream)
+{
+ struct snd_pcm_substream *s;
+
+ list_for_each_entry_reverse(s, &substream->group->substreams, link_list)
+ mutex_unlock(&s->runtime->buffer_mutex);
+ snd_pcm_group_for_each_entry(s, substream)
+ atomic_inc(&s->runtime->buffer_accessing);
}
/* fill the PCM buffer with the current silence format; called from pcm_oss.c
*/
@@ -1317,9 +1355,9 @@
};
/*
- * this functions is core for handling of linked stream
- * Note: the stream state might be changed also on failure
- * Note2: call with calling stream lock + link lock
+ * Handle an action on all linked streams.
+ * When stream_lock is true, the caller holds the calling stream and group
+ * locks. Otherwise, the caller holds link_rwsem and every group buffer_mutex.
*/
static int snd_pcm_action_group(const struct action_ops *ops,
struct snd_pcm_substream *substream,
@@ -1331,10 +1369,8 @@
int res = 0, depth = 1;
snd_pcm_group_for_each_entry(s, substream) {
- if (s != substream) {
- if (!stream_lock)
- mutex_lock_nested(&s->runtime->buffer_mutex,
depth);
- else if (s->pcm->nonatomic)
+ if (s != substream && stream_lock) {
+ if (s->pcm->nonatomic)
mutex_lock_nested(&s->self_group.mutex, depth);
else
spin_lock_nested(&s->self_group.lock, depth);
@@ -1371,10 +1407,8 @@
_unlock:
/* unlock streams */
snd_pcm_group_for_each_entry(s1, substream) {
- if (s1 != substream) {
- if (!stream_lock)
- mutex_unlock(&s1->runtime->buffer_mutex);
- else if (s1->pcm->nonatomic)
+ if (s1 != substream && stream_lock) {
+ if (s1->pcm->nonatomic)
mutex_unlock(&s1->self_group.mutex);
else
spin_unlock(&s1->self_group.lock);
@@ -1512,18 +1546,25 @@
struct snd_pcm_substream *substream,
snd_pcm_state_t state)
{
+ bool linked;
int res;
/* Guarantee the group members won't change during non-atomic action */
guard(rwsem_read)(&snd_pcm_link_rwsem);
- res = snd_pcm_buffer_access_lock(substream->runtime);
+ linked = snd_pcm_stream_linked(substream);
+ if (linked)
+ res = snd_pcm_group_buffer_access_lock(substream);
+ else
+ res = snd_pcm_buffer_access_lock(substream->runtime);
if (res < 0)
return res;
- if (snd_pcm_stream_linked(substream))
+ if (linked) {
res = snd_pcm_action_group(ops, substream, state, false);
- else
+ snd_pcm_group_buffer_access_unlock(substream);
+ } else {
res = snd_pcm_action_single(ops, substream, state, false);
- snd_pcm_buffer_access_unlock(substream->runtime);
+ snd_pcm_buffer_access_unlock(substream->runtime);
+ }
return res;
}
