[PATCH 2/8] ASoC: qcom: q6apm-dai: fix race condition while updating the position pointer

From: Srinivas Kandagatla
Date: Thu Feb 09 2023 - 07:29:17 EST


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@xxxxxxxxxx>
---
sound/soc/qcom/qdsp6/q6apm-dai.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index ee59ef36b85a..bd35067a4052 100644
--- 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,
--
2.21.0