linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] New debug feature: IPC message injector
@ 2021-11-16 15:21 Daniel Baluta
  2021-11-16 15:21 ` [PATCH 1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message Daniel Baluta
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Daniel Baluta @ 2021-11-16 15:21 UTC (permalink / raw)
  To: broonie, peter.ujfalusi
  Cc: daniel.baluta, daniel.baluta, pierre-louis.bossart, lgirdwood,
	ranjani.sridharan, kai.vehmanen, alsa-devel, linux-kernel

From: Daniel Baluta <daniel.baluta@nxp.com>

With the new SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR it is going to be
possible to inject arbitrary messages via the debugfs/sof/ipc_msg_inject
file and get the reply from the same file as a binary.

The main use of this feature is to stress test the firmware's robustness
against maliciously (or erroneous) crafted messages.
We also receive firmware crash reports with only a binary of the sent
message which caused the firmware crash.

Before adding the new debug feature we needed to make sure that the IPC
reply handling is working correctly for cases when we don't know
beforehand the size of the reply (since we don't know what kind of
message we are sending).

Peter Ujfalusi (4):
  ASoC: SOF: utils: Add generic function to get the reply for a tx
    message
  ASoC: SOF: imx: Use the generic helper to get the reply
  ASoC: SOF: intel: Use the generic helper to get the reply
  ASoC: SOF: debug: Add support for IPC message injection

 sound/soc/sof/Kconfig         |   8 +++
 sound/soc/sof/debug.c         | 107 ++++++++++++++++++++++++++++++++++
 sound/soc/sof/imx/imx8.c      |  37 +-----------
 sound/soc/sof/imx/imx8m.c     |  37 +-----------
 sound/soc/sof/intel/atom.c    |  43 +-------------
 sound/soc/sof/intel/bdw.c     |  43 +-------------
 sound/soc/sof/intel/hda-ipc.c |  29 +--------
 sound/soc/sof/ipc.c           |  61 +++++++++++++++++++
 sound/soc/sof/sof-priv.h      |  10 ++++
 9 files changed, 192 insertions(+), 183 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message
  2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
@ 2021-11-16 15:21 ` Daniel Baluta
  2021-11-16 15:21 ` [PATCH 2/4] ASoC: SOF: imx: Use the generic helper to get the reply Daniel Baluta
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Baluta @ 2021-11-16 15:21 UTC (permalink / raw)
  To: broonie, peter.ujfalusi
  Cc: daniel.baluta, daniel.baluta, pierre-louis.bossart, lgirdwood,
	ranjani.sridharan, kai.vehmanen, alsa-devel, linux-kernel,
	Rander Wang, Guennadi Liakhovetski

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

The code to get the reply for a tx is identical in all but one place:
imx8_get_reply(), imx8m_get_reply(), atom_get_reply(), bdw_get_reply().

hda_dsp_ipc_get_reply() have additional check in place for PROBES and
special handling of PM messages.

Add a generic implementation to the core which can be used as drop in
replacement.

The reply size check is changed to be able to handle cases when the reply
size is not know beforehand (this is the case for PROBES and
DEBUG_MEM_USAGE for example).

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/sof/ipc.c      | 61 ++++++++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-priv.h |  6 ++++
 2 files changed, 67 insertions(+)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index a4036d0b3d3a..6771b444065d 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -394,6 +394,67 @@ int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
 }
 EXPORT_SYMBOL(sof_ipc_tx_message_no_pm);
 
+/* Generic helper function to retrieve the reply */
+void snd_sof_ipc_get_reply(struct snd_sof_dev *sdev)
+{
+	struct snd_sof_ipc_msg *msg = sdev->msg;
+	struct sof_ipc_reply reply;
+	int ret = 0;
+
+	/*
+	 * Sometimes, there is unexpected reply ipc arriving. The reply
+	 * ipc belongs to none of the ipcs sent from driver.
+	 * In this case, the driver must ignore the ipc.
+	 */
+	if (!msg) {
+		dev_warn(sdev->dev, "unexpected ipc interrupt raised!\n");
+		return;
+	}
+
+	/* get the generic reply */
+	snd_sof_dsp_mailbox_read(sdev, sdev->host_box.offset, &reply,
+				 sizeof(reply));
+
+	if (reply.error < 0) {
+		memcpy(msg->reply_data, &reply, sizeof(reply));
+		ret = reply.error;
+	} else if (!reply.hdr.size) {
+		/* Reply should always be >= sizeof(struct sof_ipc_reply) */
+		if (msg->reply_size)
+			dev_err(sdev->dev,
+				"empty reply received, expected %zu bytes\n",
+				msg->reply_size);
+		else
+			dev_err(sdev->dev, "empty reply received\n");
+
+		ret = -EINVAL;
+	} else if (msg->reply_size > 0) {
+		if (reply.hdr.size == msg->reply_size) {
+			ret = 0;
+		} else if (reply.hdr.size < msg->reply_size) {
+			dev_dbg(sdev->dev,
+				"reply size (%u) is less than expected (%zu)\n",
+				reply.hdr.size, msg->reply_size);
+
+			msg->reply_size = reply.hdr.size;
+			ret = 0;
+		} else {
+			dev_err(sdev->dev,
+				"reply size (%u) exceeds the buffer size (%zu)\n",
+				reply.hdr.size, msg->reply_size);
+			ret = -EINVAL;
+		}
+
+		/* get the full message if reply.hdr.size <= msg->reply_size */
+		if (!ret)
+			snd_sof_dsp_mailbox_read(sdev, sdev->host_box.offset,
+						 msg->reply_data, msg->reply_size);
+	}
+
+	msg->reply_error = ret;
+}
+EXPORT_SYMBOL(snd_sof_ipc_get_reply);
+
 /* handle reply message from DSP */
 void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
 {
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index ba341b1bda0c..2c97ffa98e3e 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -515,6 +515,7 @@ void snd_sof_fw_unload(struct snd_sof_dev *sdev);
  */
 struct snd_sof_ipc *snd_sof_ipc_init(struct snd_sof_dev *sdev);
 void snd_sof_ipc_free(struct snd_sof_dev *sdev);
+void snd_sof_ipc_get_reply(struct snd_sof_dev *sdev);
 void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id);
 void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev);
 int snd_sof_ipc_stream_pcm_params(struct snd_sof_dev *sdev,
@@ -527,6 +528,11 @@ int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
 			     void *msg_data, size_t msg_bytes,
 			     void *reply_data, size_t reply_bytes);
 int sof_ipc_init_msg_memory(struct snd_sof_dev *sdev);
+static inline void snd_sof_ipc_process_reply(struct snd_sof_dev *sdev, u32 msg_id)
+{
+	snd_sof_ipc_get_reply(sdev);
+	snd_sof_ipc_reply(sdev, msg_id);
+}
 
 /*
  * Trace/debug
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] ASoC: SOF: imx: Use the generic helper to get the reply
  2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
  2021-11-16 15:21 ` [PATCH 1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message Daniel Baluta
@ 2021-11-16 15:21 ` Daniel Baluta
  2021-11-16 15:21 ` [PATCH 3/4] ASoC: SOF: intel: " Daniel Baluta
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Baluta @ 2021-11-16 15:21 UTC (permalink / raw)
  To: broonie, peter.ujfalusi
  Cc: daniel.baluta, daniel.baluta, pierre-louis.bossart, lgirdwood,
	ranjani.sridharan, kai.vehmanen, alsa-devel, linux-kernel,
	Rander Wang, Guennadi Liakhovetski

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Make use of the generic snd_sof_ipc_process_reply() from the core instead
the local implementation.
snd_sof_ipc_process_reply() handles the reply retrieving and the ipc reply

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/sof/imx/imx8.c  | 37 +------------------------------------
 sound/soc/sof/imx/imx8m.c | 37 +------------------------------------
 2 files changed, 2 insertions(+), 72 deletions(-)

diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index dd59a74480d6..0aeb44d0acc7 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -59,40 +59,6 @@ struct imx8_priv {
 
 };
 
-static void imx8_get_reply(struct snd_sof_dev *sdev)
-{
-	struct snd_sof_ipc_msg *msg = sdev->msg;
-	struct sof_ipc_reply reply;
-	int ret = 0;
-
-	if (!msg) {
-		dev_warn(sdev->dev, "unexpected ipc interrupt\n");
-		return;
-	}
-
-	/* get reply */
-	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
-
-	if (reply.error < 0) {
-		memcpy(msg->reply_data, &reply, sizeof(reply));
-		ret = reply.error;
-	} else {
-		/* reply has correct size? */
-		if (reply.hdr.size != msg->reply_size) {
-			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
-				msg->reply_size, reply.hdr.size);
-			ret = -EINVAL;
-		}
-
-		/* read the message */
-		if (msg->reply_size > 0)
-			sof_mailbox_read(sdev, sdev->host_box.offset,
-					 msg->reply_data, msg->reply_size);
-	}
-
-	msg->reply_error = ret;
-}
-
 static int imx8_get_mailbox_offset(struct snd_sof_dev *sdev)
 {
 	return MBOX_OFFSET;
@@ -109,8 +75,7 @@ static void imx8_dsp_handle_reply(struct imx_dsp_ipc *ipc)
 	unsigned long flags;
 
 	spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
-	imx8_get_reply(priv->sdev);
-	snd_sof_ipc_reply(priv->sdev, 0);
+	snd_sof_ipc_process_reply(priv->sdev, 0);
 	spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
 }
 
diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index e4618980cf8b..f454a5d0a87e 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -32,40 +32,6 @@ struct imx8m_priv {
 	struct platform_device *ipc_dev;
 };
 
-static void imx8m_get_reply(struct snd_sof_dev *sdev)
-{
-	struct snd_sof_ipc_msg *msg = sdev->msg;
-	struct sof_ipc_reply reply;
-	int ret = 0;
-
-	if (!msg) {
-		dev_warn(sdev->dev, "unexpected ipc interrupt\n");
-		return;
-	}
-
-	/* get reply */
-	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
-
-	if (reply.error < 0) {
-		memcpy(msg->reply_data, &reply, sizeof(reply));
-		ret = reply.error;
-	} else {
-		/* reply has correct size? */
-		if (reply.hdr.size != msg->reply_size) {
-			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
-				msg->reply_size, reply.hdr.size);
-			ret = -EINVAL;
-		}
-
-		/* read the message */
-		if (msg->reply_size > 0)
-			sof_mailbox_read(sdev, sdev->host_box.offset,
-					 msg->reply_data, msg->reply_size);
-	}
-
-	msg->reply_error = ret;
-}
-
 static int imx8m_get_mailbox_offset(struct snd_sof_dev *sdev)
 {
 	return MBOX_OFFSET;
@@ -82,8 +48,7 @@ static void imx8m_dsp_handle_reply(struct imx_dsp_ipc *ipc)
 	unsigned long flags;
 
 	spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
-	imx8m_get_reply(priv->sdev);
-	snd_sof_ipc_reply(priv->sdev, 0);
+	snd_sof_ipc_process_reply(priv->sdev, 0);
 	spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] ASoC: SOF: intel: Use the generic helper to get the reply
  2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
  2021-11-16 15:21 ` [PATCH 1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message Daniel Baluta
  2021-11-16 15:21 ` [PATCH 2/4] ASoC: SOF: imx: Use the generic helper to get the reply Daniel Baluta
@ 2021-11-16 15:21 ` Daniel Baluta
  2021-11-16 15:21 ` [PATCH 4/4] ASoC: SOF: debug: Add support for IPC message injection Daniel Baluta
  2021-11-17 22:31 ` [PATCH 0/4] New debug feature: IPC message injector Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Baluta @ 2021-11-16 15:21 UTC (permalink / raw)
  To: broonie, peter.ujfalusi
  Cc: daniel.baluta, daniel.baluta, pierre-louis.bossart, lgirdwood,
	ranjani.sridharan, kai.vehmanen, alsa-devel, linux-kernel,
	Rander Wang, Guennadi Liakhovetski

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Make use of the generic snd_sof_ipc_process_reply() from the core instead
the local implementation.
snd_sof_ipc_process_reply() handles the reply retrieving and the ipc reply

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/sof/intel/atom.c    | 43 +----------------------------------
 sound/soc/sof/intel/bdw.c     | 43 +----------------------------------
 sound/soc/sof/intel/hda-ipc.c | 29 ++---------------------
 3 files changed, 4 insertions(+), 111 deletions(-)

diff --git a/sound/soc/sof/intel/atom.c b/sound/soc/sof/intel/atom.c
index 74c630bb9847..cdc96a7df493 100644
--- a/sound/soc/sof/intel/atom.c
+++ b/sound/soc/sof/intel/atom.c
@@ -27,7 +27,6 @@
 
 static void atom_host_done(struct snd_sof_dev *sdev);
 static void atom_dsp_done(struct snd_sof_dev *sdev);
-static void atom_get_reply(struct snd_sof_dev *sdev);
 
 /*
  * Debug
@@ -154,8 +153,7 @@ irqreturn_t atom_irq_thread(int irq, void *context)
 		 * because the done bit can't be set in cmd_done function
 		 * which is triggered by msg
 		 */
-		atom_get_reply(sdev);
-		snd_sof_ipc_reply(sdev, ipcx);
+		snd_sof_ipc_process_reply(sdev, ipcx);
 
 		atom_dsp_done(sdev);
 
@@ -195,45 +193,6 @@ int atom_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
 }
 EXPORT_SYMBOL_NS(atom_send_msg, SND_SOC_SOF_INTEL_ATOM_HIFI_EP);
 
-static void atom_get_reply(struct snd_sof_dev *sdev)
-{
-	struct snd_sof_ipc_msg *msg = sdev->msg;
-	struct sof_ipc_reply reply;
-	int ret = 0;
-
-	/*
-	 * Sometimes, there is unexpected reply ipc arriving. The reply
-	 * ipc belongs to none of the ipcs sent from driver.
-	 * In this case, the driver must ignore the ipc.
-	 */
-	if (!msg) {
-		dev_warn(sdev->dev, "unexpected ipc interrupt raised!\n");
-		return;
-	}
-
-	/* get reply */
-	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
-
-	if (reply.error < 0) {
-		memcpy(msg->reply_data, &reply, sizeof(reply));
-		ret = reply.error;
-	} else {
-		/* reply correct size ? */
-		if (reply.hdr.size != msg->reply_size) {
-			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
-				msg->reply_size, reply.hdr.size);
-			ret = -EINVAL;
-		}
-
-		/* read the message */
-		if (msg->reply_size > 0)
-			sof_mailbox_read(sdev, sdev->host_box.offset,
-					 msg->reply_data, msg->reply_size);
-	}
-
-	msg->reply_error = ret;
-}
-
 int atom_get_mailbox_offset(struct snd_sof_dev *sdev)
 {
 	return MBOX_OFFSET;
diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c
index 2c09a523288e..156006bed017 100644
--- a/sound/soc/sof/intel/bdw.c
+++ b/sound/soc/sof/intel/bdw.c
@@ -75,7 +75,6 @@ static const struct snd_sof_debugfs_map bdw_debugfs[] = {
 
 static void bdw_host_done(struct snd_sof_dev *sdev);
 static void bdw_dsp_done(struct snd_sof_dev *sdev);
-static void bdw_get_reply(struct snd_sof_dev *sdev);
 
 /*
  * DSP Control.
@@ -326,8 +325,7 @@ static irqreturn_t bdw_irq_thread(int irq, void *context)
 		 * because the done bit can't be set in cmd_done function
 		 * which is triggered by msg
 		 */
-		bdw_get_reply(sdev);
-		snd_sof_ipc_reply(sdev, ipcx);
+		snd_sof_ipc_process_reply(sdev, ipcx);
 
 		bdw_dsp_done(sdev);
 
@@ -372,45 +370,6 @@ static int bdw_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
 	return 0;
 }
 
-static void bdw_get_reply(struct snd_sof_dev *sdev)
-{
-	struct snd_sof_ipc_msg *msg = sdev->msg;
-	struct sof_ipc_reply reply;
-	int ret = 0;
-
-	/*
-	 * Sometimes, there is unexpected reply ipc arriving. The reply
-	 * ipc belongs to none of the ipcs sent from driver.
-	 * In this case, the driver must ignore the ipc.
-	 */
-	if (!msg) {
-		dev_warn(sdev->dev, "unexpected ipc interrupt raised!\n");
-		return;
-	}
-
-	/* get reply */
-	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
-
-	if (reply.error < 0) {
-		memcpy(msg->reply_data, &reply, sizeof(reply));
-		ret = reply.error;
-	} else {
-		/* reply correct size ? */
-		if (reply.hdr.size != msg->reply_size) {
-			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
-				msg->reply_size, reply.hdr.size);
-			ret = -EINVAL;
-		}
-
-		/* read the message */
-		if (msg->reply_size > 0)
-			sof_mailbox_read(sdev, sdev->host_box.offset,
-					 msg->reply_data, msg->reply_size);
-	}
-
-	msg->reply_error = ret;
-}
-
 static int bdw_get_mailbox_offset(struct snd_sof_dev *sdev)
 {
 	return MBOX_OFFSET;
diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c
index 11f20a5a62df..2019087a84ce 100644
--- a/sound/soc/sof/intel/hda-ipc.c
+++ b/sound/soc/sof/intel/hda-ipc.c
@@ -70,7 +70,6 @@ void hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev)
 	struct snd_sof_ipc_msg *msg = sdev->msg;
 	struct sof_ipc_reply reply;
 	struct sof_ipc_cmd_hdr *hdr;
-	int ret = 0;
 
 	/*
 	 * Sometimes, there is unexpected reply ipc arriving. The reply
@@ -94,35 +93,11 @@ void hda_dsp_ipc_get_reply(struct snd_sof_dev *sdev)
 		reply.hdr.cmd = SOF_IPC_GLB_REPLY;
 		reply.hdr.size = sizeof(reply);
 		memcpy(msg->reply_data, &reply, sizeof(reply));
-		goto out;
-	}
-
-	/* get IPC reply from DSP in the mailbox */
-	sof_mailbox_read(sdev, sdev->host_box.offset, &reply,
-			 sizeof(reply));
 
-	if (reply.error < 0) {
-		memcpy(msg->reply_data, &reply, sizeof(reply));
-		ret = reply.error;
+		msg->reply_error = 0;
 	} else {
-		/* reply correct size ? */
-		if (reply.hdr.size != msg->reply_size &&
-		    /* getter payload is never known upfront */
-		    ((reply.hdr.cmd & SOF_GLB_TYPE_MASK) != SOF_IPC_GLB_PROBE)) {
-			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
-				msg->reply_size, reply.hdr.size);
-			ret = -EINVAL;
-		}
-
-		/* read the message */
-		if (msg->reply_size > 0)
-			sof_mailbox_read(sdev, sdev->host_box.offset,
-					 msg->reply_data, msg->reply_size);
+		snd_sof_ipc_get_reply(sdev);
 	}
-
-out:
-	msg->reply_error = ret;
-
 }
 
 /* IPC handler thread */
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] ASoC: SOF: debug: Add support for IPC message injection
  2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
                   ` (2 preceding siblings ...)
  2021-11-16 15:21 ` [PATCH 3/4] ASoC: SOF: intel: " Daniel Baluta
@ 2021-11-16 15:21 ` Daniel Baluta
  2021-11-17 22:31 ` [PATCH 0/4] New debug feature: IPC message injector Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Baluta @ 2021-11-16 15:21 UTC (permalink / raw)
  To: broonie, peter.ujfalusi
  Cc: daniel.baluta, daniel.baluta, pierre-louis.bossart, lgirdwood,
	ranjani.sridharan, kai.vehmanen, alsa-devel, linux-kernel,
	Rander Wang, Guennadi Liakhovetski

From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

In order to stress test the firmware's ability to handle (mis)crafted
IPC messages this patch adds a debugfs interface where a binary file
(message) can be written and the message is sent to the firmware as it is.

Read on the same file will return the reply from the firmware if it is
available as a binary.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/sof/Kconfig    |   8 +++
 sound/soc/sof/debug.c    | 107 +++++++++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-priv.h |   4 ++
 3 files changed, 119 insertions(+)

diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index b6fa659179b6..89eea5558190 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -194,6 +194,14 @@ config SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST
 	  Say Y if you want to enable IPC flood test.
 	  If unsure, select "N".
 
+config SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR
+	bool "SOF enable IPC message injector"
+	help
+	  This option enables the IPC message injector which can be used to send
+	  crafted IPC messages to the DSP to test its robustness.
+	  Say Y if you want to enable the IPC message injector.
+	  If unsure, select "N".
+
 config SND_SOC_SOF_DEBUG_RETAIN_DSP_CONTEXT
 	bool "SOF retain DSP context on any FW exceptions"
 	help
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index dc1df5fb7b4c..2f8b5ac9b78a 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -336,6 +336,104 @@ static int sof_debug_ipc_flood_test(struct snd_sof_dev *sdev,
 }
 #endif
 
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR)
+static ssize_t msg_inject_read(struct file *file, char __user *buffer,
+			       size_t count, loff_t *ppos)
+{
+	struct snd_sof_dfsentry *dfse = file->private_data;
+	struct sof_ipc_reply *rhdr = dfse->msg_inject_rx;
+
+	if (!rhdr->hdr.size || !count || *ppos)
+		return 0;
+
+	if (count > rhdr->hdr.size)
+		count = rhdr->hdr.size;
+
+	if (copy_to_user(buffer, dfse->msg_inject_rx, count))
+		return -EFAULT;
+
+	*ppos += count;
+	return count;
+}
+
+static ssize_t msg_inject_write(struct file *file, const char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	struct snd_sof_dfsentry *dfse = file->private_data;
+	struct snd_sof_dev *sdev = dfse->sdev;
+	struct sof_ipc_cmd_hdr *hdr = dfse->msg_inject_tx;
+	size_t size;
+	int ret, err;
+
+	if (*ppos)
+		return 0;
+
+	size = simple_write_to_buffer(dfse->msg_inject_tx, SOF_IPC_MSG_MAX_SIZE,
+				      ppos, buffer, count);
+	if (size != count)
+		return size > 0 ? -EFAULT : size;
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0 && ret != -EACCES) {
+		dev_err_ratelimited(sdev->dev, "%s: DSP resume failed: %d\n",
+				    __func__, ret);
+		pm_runtime_put_noidle(sdev->dev);
+		goto out;
+	}
+
+	/* send the message */
+	memset(dfse->msg_inject_rx, 0, SOF_IPC_MSG_MAX_SIZE);
+	ret = sof_ipc_tx_message(sdev->ipc, hdr->cmd, dfse->msg_inject_tx, count,
+				 dfse->msg_inject_rx, SOF_IPC_MSG_MAX_SIZE);
+
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "%s: DSP idle failed: %d\n",
+				    __func__, err);
+
+	/* return size if test is successful */
+	if (ret >= 0)
+		ret = size;
+
+out:
+	return ret;
+}
+
+static const struct file_operations msg_inject_fops = {
+	.open = simple_open,
+	.read = msg_inject_read,
+	.write = msg_inject_write,
+	.llseek = default_llseek,
+};
+
+static int snd_sof_debugfs_msg_inject_item(struct snd_sof_dev *sdev,
+					   const char *name, mode_t mode,
+					   const struct file_operations *fops)
+{
+	struct snd_sof_dfsentry *dfse;
+
+	dfse = devm_kzalloc(sdev->dev, sizeof(*dfse), GFP_KERNEL);
+	if (!dfse)
+		return -ENOMEM;
+
+	/* pre allocate the tx and rx buffers */
+	dfse->msg_inject_tx = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL);
+	dfse->msg_inject_rx = devm_kzalloc(sdev->dev, SOF_IPC_MSG_MAX_SIZE, GFP_KERNEL);
+	if (!dfse->msg_inject_tx || !dfse->msg_inject_rx)
+		return -ENOMEM;
+
+	dfse->type = SOF_DFSENTRY_TYPE_BUF;
+	dfse->sdev = sdev;
+
+	debugfs_create_file(name, mode, sdev->debugfs_root, dfse, fops);
+	/* add to dfsentry list */
+	list_add(&dfse->list, &sdev->dfsentry_list);
+
+	return 0;
+}
+#endif
+
 static ssize_t sof_dfsentry_write(struct file *file, const char __user *buffer,
 				  size_t count, loff_t *ppos)
 {
@@ -812,6 +910,15 @@ int snd_sof_dbg_init(struct snd_sof_dev *sdev)
 		return err;
 #endif
 
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR)
+	err = snd_sof_debugfs_msg_inject_item(sdev, "ipc_msg_inject", 0644,
+					      &msg_inject_fops);
+
+	/* errors are only due to memory allocation, not debugfs */
+	if (err < 0)
+		return err;
+#endif
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_sof_dbg_init);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 2c97ffa98e3e..9a8af76b2f8b 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -325,6 +325,10 @@ struct snd_sof_dfsentry {
 	enum sof_debugfs_access_type access_type;
 #if ENABLE_DEBUGFS_CACHEBUF
 	char *cache_buf; /* buffer to cache the contents of debugfs memory */
+#endif
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR)
+	void *msg_inject_tx;
+	void *msg_inject_rx;
 #endif
 	struct snd_sof_dev *sdev;
 	struct list_head list;  /* list in sdev dfsentry list */
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] New debug feature: IPC message injector
  2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
                   ` (3 preceding siblings ...)
  2021-11-16 15:21 ` [PATCH 4/4] ASoC: SOF: debug: Add support for IPC message injection Daniel Baluta
@ 2021-11-17 22:31 ` Mark Brown
  4 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2021-11-17 22:31 UTC (permalink / raw)
  To: Daniel Baluta, peter.ujfalusi
  Cc: linux-kernel, kai.vehmanen, alsa-devel, daniel.baluta, lgirdwood,
	daniel.baluta, pierre-louis.bossart, ranjani.sridharan

On Tue, 16 Nov 2021 17:21:33 +0200, Daniel Baluta wrote:
> From: Daniel Baluta <daniel.baluta@nxp.com>
> 
> With the new SND_SOC_SOF_DEBUG_IPC_MSG_INJECTOR it is going to be
> possible to inject arbitrary messages via the debugfs/sof/ipc_msg_inject
> file and get the reply from the same file as a binary.
> 
> The main use of this feature is to stress test the firmware's robustness
> against maliciously (or erroneous) crafted messages.
> We also receive firmware crash reports with only a binary of the sent
> message which caused the firmware crash.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message
      commit: 8ae77801c81d16a09f6b67a6f8d91255d34f5f2c
[2/4] ASoC: SOF: imx: Use the generic helper to get the reply
      commit: 18c45f270352fb76c8b5b133b3ae3971769f8a22
[3/4] ASoC: SOF: intel: Use the generic helper to get the reply
      commit: 0bd2891bda4550774946abbfac88443a16c15d5a
[4/4] ASoC: SOF: debug: Add support for IPC message injection
      commit: 2f0b1b013bbc5d6f4c7c386e12f423d6b4ef3245

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-11-17 22:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-16 15:21 [PATCH 0/4] New debug feature: IPC message injector Daniel Baluta
2021-11-16 15:21 ` [PATCH 1/4] ASoC: SOF: utils: Add generic function to get the reply for a tx message Daniel Baluta
2021-11-16 15:21 ` [PATCH 2/4] ASoC: SOF: imx: Use the generic helper to get the reply Daniel Baluta
2021-11-16 15:21 ` [PATCH 3/4] ASoC: SOF: intel: " Daniel Baluta
2021-11-16 15:21 ` [PATCH 4/4] ASoC: SOF: debug: Add support for IPC message injection Daniel Baluta
2021-11-17 22:31 ` [PATCH 0/4] New debug feature: IPC message injector Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).