All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Sasha Levin <sashal@kernel.org>,
	guennadi.liakhovetski@linux.intel.com,
	alsa-devel@alsa-project.org,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	lgirdwood@gmail.com, rander.wang@intel.com,
	yung-chuan.liao@linux.intel.com, tiwai@suse.com,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	daniel.baluta@nxp.com,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Mark Brown <broonie@kernel.org>,
	Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
	sound-open-firmware@alsa-project.org
Subject: [PATCH AUTOSEL 5.19 13/48] ASoC: SOF: Intel: hda-ipc: Do not process IPC reply before firmware boot
Date: Sun, 14 Aug 2022 12:19:06 -0400	[thread overview]
Message-ID: <20220814161943.2394452-13-sashal@kernel.org> (raw)
In-Reply-To: <20220814161943.2394452-1-sashal@kernel.org>

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

[ Upstream commit 499cc881b09c8283ab5e75b0d6d21cb427722161 ]

It is not yet clear, but it is possible to create a firmware so broken
that it will send a reply message before a FW_READY message (it is not
yet clear if FW_READY will arrive later).
Since the reply_data is allocated only after the FW_READY message, this
will lead to a NULL pointer dereference if not filtered out.

The issue was reported with IPC4 firmware but the same condition is present
for IPC3.

Reported-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20220712122357.31282-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 sound/soc/sof/intel/hda-ipc.c | 39 ++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c
index f08011249955..65e688f749ea 100644
--- a/sound/soc/sof/intel/hda-ipc.c
+++ b/sound/soc/sof/intel/hda-ipc.c
@@ -148,17 +148,23 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context)
 
 		if (primary & SOF_IPC4_MSG_DIR_MASK) {
 			/* Reply received */
-			struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data;
+			if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) {
+				struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data;
 
-			data->primary = primary;
-			data->extension = extension;
+				data->primary = primary;
+				data->extension = extension;
 
-			spin_lock_irq(&sdev->ipc_lock);
+				spin_lock_irq(&sdev->ipc_lock);
 
-			snd_sof_ipc_get_reply(sdev);
-			snd_sof_ipc_reply(sdev, data->primary);
+				snd_sof_ipc_get_reply(sdev);
+				snd_sof_ipc_reply(sdev, data->primary);
 
-			spin_unlock_irq(&sdev->ipc_lock);
+				spin_unlock_irq(&sdev->ipc_lock);
+			} else {
+				dev_dbg_ratelimited(sdev->dev,
+						    "IPC reply before FW_READY: %#x|%#x\n",
+						    primary, extension);
+			}
 		} else {
 			/* Notification received */
 
@@ -225,16 +231,21 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
 		 * place, the message might not yet be marked as expecting a
 		 * reply.
 		 */
-		spin_lock_irq(&sdev->ipc_lock);
+		if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) {
+			spin_lock_irq(&sdev->ipc_lock);
 
-		/* handle immediate reply from DSP core */
-		hda_dsp_ipc_get_reply(sdev);
-		snd_sof_ipc_reply(sdev, msg);
+			/* handle immediate reply from DSP core */
+			hda_dsp_ipc_get_reply(sdev);
+			snd_sof_ipc_reply(sdev, msg);
 
-		/* set the done bit */
-		hda_dsp_ipc_dsp_done(sdev);
+			/* set the done bit */
+			hda_dsp_ipc_dsp_done(sdev);
 
-		spin_unlock_irq(&sdev->ipc_lock);
+			spin_unlock_irq(&sdev->ipc_lock);
+		} else {
+			dev_dbg_ratelimited(sdev->dev, "IPC reply before FW_READY: %#x\n",
+					    msg);
+		}
 
 		ipc_irq = true;
 	}
-- 
2.35.1


WARNING: multiple messages have this Message-ID (diff)
From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	Mark Brown <broonie@kernel.org>, Sasha Levin <sashal@kernel.org>,
	lgirdwood@gmail.com, yung-chuan.liao@linux.intel.com,
	daniel.baluta@nxp.com, perex@perex.cz, tiwai@suse.com,
	rander.wang@intel.com, guennadi.liakhovetski@linux.intel.com,
	sound-open-firmware@alsa-project.org,
	alsa-devel@alsa-project.org
Subject: [PATCH AUTOSEL 5.19 13/48] ASoC: SOF: Intel: hda-ipc: Do not process IPC reply before firmware boot
Date: Sun, 14 Aug 2022 12:19:06 -0400	[thread overview]
Message-ID: <20220814161943.2394452-13-sashal@kernel.org> (raw)
In-Reply-To: <20220814161943.2394452-1-sashal@kernel.org>

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

[ Upstream commit 499cc881b09c8283ab5e75b0d6d21cb427722161 ]

It is not yet clear, but it is possible to create a firmware so broken
that it will send a reply message before a FW_READY message (it is not
yet clear if FW_READY will arrive later).
Since the reply_data is allocated only after the FW_READY message, this
will lead to a NULL pointer dereference if not filtered out.

The issue was reported with IPC4 firmware but the same condition is present
for IPC3.

Reported-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20220712122357.31282-3-peter.ujfalusi@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 sound/soc/sof/intel/hda-ipc.c | 39 ++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/sound/soc/sof/intel/hda-ipc.c b/sound/soc/sof/intel/hda-ipc.c
index f08011249955..65e688f749ea 100644
--- a/sound/soc/sof/intel/hda-ipc.c
+++ b/sound/soc/sof/intel/hda-ipc.c
@@ -148,17 +148,23 @@ irqreturn_t hda_dsp_ipc4_irq_thread(int irq, void *context)
 
 		if (primary & SOF_IPC4_MSG_DIR_MASK) {
 			/* Reply received */
-			struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data;
+			if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) {
+				struct sof_ipc4_msg *data = sdev->ipc->msg.reply_data;
 
-			data->primary = primary;
-			data->extension = extension;
+				data->primary = primary;
+				data->extension = extension;
 
-			spin_lock_irq(&sdev->ipc_lock);
+				spin_lock_irq(&sdev->ipc_lock);
 
-			snd_sof_ipc_get_reply(sdev);
-			snd_sof_ipc_reply(sdev, data->primary);
+				snd_sof_ipc_get_reply(sdev);
+				snd_sof_ipc_reply(sdev, data->primary);
 
-			spin_unlock_irq(&sdev->ipc_lock);
+				spin_unlock_irq(&sdev->ipc_lock);
+			} else {
+				dev_dbg_ratelimited(sdev->dev,
+						    "IPC reply before FW_READY: %#x|%#x\n",
+						    primary, extension);
+			}
 		} else {
 			/* Notification received */
 
@@ -225,16 +231,21 @@ irqreturn_t hda_dsp_ipc_irq_thread(int irq, void *context)
 		 * place, the message might not yet be marked as expecting a
 		 * reply.
 		 */
-		spin_lock_irq(&sdev->ipc_lock);
+		if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) {
+			spin_lock_irq(&sdev->ipc_lock);
 
-		/* handle immediate reply from DSP core */
-		hda_dsp_ipc_get_reply(sdev);
-		snd_sof_ipc_reply(sdev, msg);
+			/* handle immediate reply from DSP core */
+			hda_dsp_ipc_get_reply(sdev);
+			snd_sof_ipc_reply(sdev, msg);
 
-		/* set the done bit */
-		hda_dsp_ipc_dsp_done(sdev);
+			/* set the done bit */
+			hda_dsp_ipc_dsp_done(sdev);
 
-		spin_unlock_irq(&sdev->ipc_lock);
+			spin_unlock_irq(&sdev->ipc_lock);
+		} else {
+			dev_dbg_ratelimited(sdev->dev, "IPC reply before FW_READY: %#x\n",
+					    msg);
+		}
 
 		ipc_irq = true;
 	}
-- 
2.35.1


  parent reply	other threads:[~2022-08-14 16:22 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-14 16:18 [PATCH AUTOSEL 5.19 01/48] lib/list_debug.c: Detect uninitialized lists Sasha Levin
2022-08-14 16:18 ` [PATCH AUTOSEL 5.19 02/48] swiotlb: panic if nslabs is too small Sasha Levin
2022-08-14 16:18 ` [PATCH AUTOSEL 5.19 03/48] tty: serial: Fix refcount leak bug in ucc_uart.c Sasha Levin
2022-08-14 16:18   ` Sasha Levin
2022-08-14 16:18 ` [PATCH AUTOSEL 5.19 04/48] KVM: PPC: Book3S HV: Fix "rm_exit" entry in debugfs timings Sasha Levin
2022-08-14 16:18   ` Sasha Levin
2022-08-14 16:18 ` [PATCH AUTOSEL 5.19 05/48] vfio: Clear the caps->buf to NULL after free Sasha Levin
2022-08-14 16:18 ` [PATCH AUTOSEL 5.19 06/48] mips: cavium-octeon: Fix missing of_node_put() in octeon2_usb_clocks_start Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 07/48] iommu/io-pgtable-arm-v7s: Add a quirk to allow pgtable PA up to 35bit Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 08/48] riscv: dts: microchip: Add mpfs' topology information Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:31   ` Conor.Dooley
2022-08-14 16:31     ` Conor.Dooley
2022-08-20 14:33     ` Sasha Levin
2022-08-20 14:33       ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 09/48] ASoC: Intel: avs: Set max DMA segment size Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 10/48] ALSA: hda: Fix page fault in snd_hda_codec_shutdown() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 11/48] modules: Ensure natural alignment for .altinstructions and __bug_table sections Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 12/48] ASoC: SOF: Intel: cnl: Do not process IPC reply before firmware boot Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` Sasha Levin [this message]
2022-08-14 16:19   ` [PATCH AUTOSEL 5.19 13/48] ASoC: SOF: Intel: hda-ipc: " Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 14/48] ASoC: SOF: sof-client-probes: Only load the driver if IPC3 is used Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 15/48] ASoC: rsnd: care default case on rsnd_ssiu_busif_err_irq_ctrl() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 16/48] riscv: dts: sifive: Add fu540 topology information Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 17/48] riscv: dts: sifive: Add fu740 " Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 18/48] riscv: dts: canaan: Add k210 " Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 19/48] ASoC: nau8821: Don't unconditionally free interrupt Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 20/48] riscv: mmap with PROT_WRITE but no PROT_READ is invalid Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 21/48] RISC-V: Add fast call path of crash_kexec() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 22/48] ASoC: Intel: avs: Use lookup table to create modules Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 23/48] ALSA: hda/realtek: Enable speaker and mute LEDs for HP laptops Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 24/48] ASoC: SOF: Intel: hda: add sanity check on SSP index reported by NHLT Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 25/48] ASoC: Intel: sof_es8336: Fix GPIO quirks set via module option Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 26/48] ASoC: Intel: sof_es8336: ignore GpioInt when looking for speaker/headset GPIO lines Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 27/48] ASoC: Intel: sof_nau8825: Move quirk check to the front in late probe Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 28/48] watchdog: export lockup_detector_reconfigure Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 29/48] powerpc/watchdog: introduce a NMI watchdog's factor Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 30/48] powerpc/pseries/mobility: set NMI watchdog factor during an LPM Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 31/48] powerpc/32: Set an IBAT covering up to _einittext during init Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 32/48] powerpc/32: Don't always pass -mcpu=powerpc to the compiler Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 33/48] ASoC: codecs: va-macro: use fsgen as clock Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 34/48] ovl: warn if trusted xattr creation fails Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 35/48] powerpc/ioda/iommu/debugfs: Generate unique debugfs entries Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 36/48] ALSA: core: Add async signal helpers Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 37/48] ALSA: timer: Use deferred fasync helper Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 38/48] ALSA: pcm: " Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 39/48] ALSA: control: " Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [f2fs-dev] [PATCH AUTOSEL 5.19 40/48] f2fs: fix to avoid use f2fs_bug_on() in f2fs_new_node_page() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [f2fs-dev] [PATCH AUTOSEL 5.19 41/48] f2fs: fix to do sanity check on segment type in build_sit_entries() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 42/48] smb3: check xattr value length earlier Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 43/48] powerpc/64: Init jump labels before parse_early_param() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 44/48] venus: pm_helpers: Fix warning in OPP during probe Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 45/48] video: fbdev: i740fb: Check the argument of i740_calc_vclk() Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [PATCH AUTOSEL 5.19 46/48] MIPS: tlbex: Explicitly compare _PAGE_NO_EXEC against 0 Sasha Levin
2022-08-14 16:19 ` [f2fs-dev] [PATCH AUTOSEL 5.19 47/48] f2fs: revive F2FS_IOC_ABORT_VOLATILE_WRITE Sasha Levin
2022-08-14 16:19   ` Sasha Levin
2022-08-14 16:19 ` [f2fs-dev] [PATCH AUTOSEL 5.19 48/48] f2fs: fix null-ptr-deref in f2fs_get_dnode_of_data Sasha Levin
2022-08-14 16:19   ` Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220814161943.2394452-13-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=daniel.baluta@nxp.com \
    --cc=guennadi.liakhovetski@linux.intel.com \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter.ujfalusi@linux.intel.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rander.wang@intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=sound-open-firmware@alsa-project.org \
    --cc=stable@vger.kernel.org \
    --cc=tiwai@suse.com \
    --cc=yung-chuan.liao@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.