diff mbox

[1/3] ASoC: Intel: Skylake: Fix IPC rx_list corruption

Message ID 1494857671-19257-1-git-send-email-subhransu.s.prusty@intel.com (mailing list archive)
State Accepted
Commit 5cd1f5c32132101955d7f0e1955249a84f9b6fd9
Headers show

Commit Message

Subhransu S. Prusty May 15, 2017, 2:14 p.m. UTC 
From: Pardha Saradhi K <pardha.saradhi.kesapragada@intel.com>

In SKL+ platforms, all IPC commands are serialised, i.e. the driver sends
a new IPC to DSP, only after receiving a reply from the firmware for the
current IPC.

Hence it seems apparent that there is only a single modifier of the IPC RX
List. However, during an IPC timeout case in a multithreaded environment,
there is a possibility of the list element being deleted two times if not
properly protected.

So, use spin lock save/restore to prevent rx_list corruption.

Signed-off-by: Pardha Saradhi K <pardha.saradhi.kesapragada@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/skylake/skl-sst-ipc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Vinod Koul May 16, 2017, 6:11 a.m. UTC | #1 
On Mon, May 15, 2017 at 07:44:29PM +0530, Subhransu S. Prusty wrote:
> From: Pardha Saradhi K <pardha.saradhi.kesapragada@intel.com>
> 
> In SKL+ platforms, all IPC commands are serialised, i.e. the driver sends
> a new IPC to DSP, only after receiving a reply from the firmware for the
> current IPC.
> 
> Hence it seems apparent that there is only a single modifier of the IPC RX
> List. However, during an IPC timeout case in a multithreaded environment,
> there is a possibility of the list element being deleted two times if not
> properly protected.
> 
> So, use spin lock save/restore to prevent rx_list corruption.

Looks good, all three:

Acked-by: Vinod Koul <vinod.koul@intel.com>
diff mbox

Patch

diff --git a/sound/soc/intel/skylake/skl-sst-ipc.c b/sound/soc/intel/skylake/skl-sst-ipc.c
index 58c525096a7c..498b15345b1a 100644
--- a/sound/soc/intel/skylake/skl-sst-ipc.c
+++ b/sound/soc/intel/skylake/skl-sst-ipc.c
@@ -413,8 +413,11 @@  static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 	u32 reply = header.primary & IPC_GLB_REPLY_STATUS_MASK;
 	u64 *ipc_header = (u64 *)(&header);
 	struct skl_sst *skl = container_of(ipc, struct skl_sst, ipc);
+	unsigned long flags;
 
+	spin_lock_irqsave(&ipc->dsp->spinlock, flags);
 	msg = skl_ipc_reply_get_msg(ipc, *ipc_header);
+	spin_unlock_irqrestore(&ipc->dsp->spinlock, flags);
 	if (msg == NULL) {
 		dev_dbg(ipc->dev, "ipc: rx list is empty\n");
 		return;
@@ -456,8 +459,10 @@  static void skl_ipc_process_reply(struct sst_generic_ipc *ipc,
 		}
 	}
 
+	spin_lock_irqsave(&ipc->dsp->spinlock, flags);
 	list_del(&msg->list);
 	sst_ipc_tx_msg_reply_complete(ipc, msg);
+	spin_unlock_irqrestore(&ipc->dsp->spinlock, flags);
 }
 
 irqreturn_t skl_dsp_irq_thread_handler(int irq, void *context)