ASoC: qcom: q6apm-dai: fix race condition while updating the position pointer

It is noticed that the position pointer value seems to get a get corrupted
due to missing locking between updating and reading.

Fix this by adding a spinlock around the position pointer.

Fixes: 9b4fe0f1cd79 ("ASoC: qdsp6: audioreach: add q6apm-dai support")
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://lore.kernel.org/r/20230209122806.18923-3-srinivas.kandagatla@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index ee59ef3..bd35067 100644 (file)
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <sound/soc.h>
 #include <sound/soc-dapm.h>
+#include <linux/spinlock.h>
 #include <sound/pcm.h>
 #include <asm/dma.h>
 #include <linux/dma-mapping.h>
@@ -53,6 +54,7 @@ struct q6apm_dai_rtd {
        uint16_t session_id;
        enum stream_state state;
        struct q6apm_graph *graph;
+       spinlock_t lock;
 };
 
 struct q6apm_dai_data {
@@ -99,20 +101,25 @@ static void event_handler(uint32_t opcode, uint32_t token, uint32_t *payload, vo
 {
        struct q6apm_dai_rtd *prtd = priv;
        struct snd_pcm_substream *substream = prtd->substream;
+       unsigned long flags;
 
        switch (opcode) {
        case APM_CLIENT_EVENT_CMD_EOS_DONE:
                prtd->state = Q6APM_STREAM_STOPPED;
                break;
        case APM_CLIENT_EVENT_DATA_WRITE_DONE:
+               spin_lock_irqsave(&prtd->lock, flags);
                prtd->pos += prtd->pcm_count;
+               spin_unlock_irqrestore(&prtd->lock, flags);
                snd_pcm_period_elapsed(substream);
                if (prtd->state == Q6APM_STREAM_RUNNING)
                        q6apm_write_async(prtd->graph, prtd->pcm_count, 0, 0, 0);
 
                break;
        case APM_CLIENT_EVENT_DATA_READ_DONE:
+               spin_lock_irqsave(&prtd->lock, flags);
                prtd->pos += prtd->pcm_count;
+               spin_unlock_irqrestore(&prtd->lock, flags);
                snd_pcm_period_elapsed(substream);
                if (prtd->state == Q6APM_STREAM_RUNNING)
                        q6apm_read(prtd->graph);
@@ -253,6 +260,7 @@ static int q6apm_dai_open(struct snd_soc_component *component,
        if (prtd == NULL)
                return -ENOMEM;
 
+       spin_lock_init(&prtd->lock);
        prtd->substream = substream;
        prtd->graph = q6apm_graph_open(dev, (q6apm_cb)event_handler, prtd, graph_id);
        if (IS_ERR(prtd->graph)) {
@@ -332,11 +340,17 @@ static snd_pcm_uframes_t q6apm_dai_pointer(struct snd_soc_component *component,
 {
        struct snd_pcm_runtime *runtime = substream->runtime;
        struct q6apm_dai_rtd *prtd = runtime->private_data;
+       snd_pcm_uframes_t ptr;
+       unsigned long flags;
 
+       spin_lock_irqsave(&prtd->lock, flags);
        if (prtd->pos == prtd->pcm_size)
                prtd->pos = 0;
 
-       return bytes_to_frames(runtime, prtd->pos);
+       ptr =  bytes_to_frames(runtime, prtd->pos);
+       spin_unlock_irqrestore(&prtd->lock, flags);
+
+       return ptr;
 }
 
 static int q6apm_dai_hw_params(struct snd_soc_component *component,