From 7d3fb78b550301e43fdc60312aed733069694426 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Thu, 4 Jun 2026 11:12:44 +0100 Subject: [PATCH 1/6] ASoC: wm_adsp: Fix NULL dereference when removing firmware controls In wm_adsp_control_remove() check that the priv pointer is not NULL before attempting to cleanup what it points to. When cs_dsp creates a control it calls wm_adsp_control_add_cb() so that wm_adsp can create its own private control data. There are two cases where private data is not created: 1. The control is a SYSTEM control, so an ALSA control is not created. 2. The codec driver has registered a control_add() callback that hides the control, so wm_adsp_control_add() is not called. When cs_dsp_remove destroys its control list it calls wm_adsp_control_remove() for each control. But wm_adsp_control_remove() was attempting to cleanup the private data pointed to by cs_ctl->priv without checking the pointer for NULL. Signed-off-by: Richard Fitzgerald Fixes: 0700bc2fb94c ("ASoC: wm_adsp: Separate generic cs_dsp_coeff_ctl handling") Link: https://patch.msgid.link/20260604101244.1402862-1-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/wm_adsp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index a637e22c3929..ca630c9948e4 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -679,6 +679,9 @@ static void wm_adsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl) { struct wm_coeff_ctl *ctl = cs_ctl->priv; + if (!ctl) + return; + cancel_work_sync(&ctl->work); kfree(ctl->name); From 6042c91df60e825625bc7d5c5c3b5a87b91d5805 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda Date: Tue, 9 Jun 2026 21:38:44 +0530 Subject: [PATCH 2/6] ASoC: SOF: amd: fix for ipc flags check Firmware will set dsp_ack to 1 when firmware sends response for the IPC command issued by host. Similarly dsp_msg flag will be updated to 1. During ACP D0 entry, the value read from the sof_dsp_ack_write scratch flag can be uninitialized. A non-zero garbage value is treated as a pending DSP IPC ack before SOF_FW_BOOT_COMPLETE, causing a spurious "IPC reply before FW_BOOT_COMPLETE" log. Fix the condition checks for ipc flags. Fixes: 738a2b5e2cc9 ("ASoC: SOF: amd: Add IPC support for ACP IP block") Link: https://github.com/thesofproject/linux/pull/5642 Signed-off-by: Vijendar Mukunda Tested-by: Umang Jain Link: https://patch.msgid.link/20260609160938.3717513-1-Vijendar.Mukunda@amd.com Signed-off-by: Mark Brown --- sound/soc/sof/amd/acp-ipc.c | 4 ++-- sound/soc/sof/amd/acp.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/amd/acp-ipc.c b/sound/soc/sof/amd/acp-ipc.c index 3cd4674dd800..94025bc799ea 100644 --- a/sound/soc/sof/amd/acp-ipc.c +++ b/sound/soc/sof/amd/acp-ipc.c @@ -181,14 +181,14 @@ irqreturn_t acp_sof_ipc_irq_thread(int irq, void *context) } dsp_msg = snd_sof_dsp_read(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + dsp_msg_write); - if (dsp_msg) { + if (dsp_msg == ACP_DSP_MSG_SET) { snd_sof_ipc_msgs_rx(sdev); acp_dsp_ipc_host_done(sdev); ipc_irq = true; } dsp_ack = snd_sof_dsp_read(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + dsp_ack_write); - if (dsp_ack) { + if (dsp_ack == ACP_DSP_ACK_SET) { if (likely(sdev->fw_state == SOF_FW_BOOT_COMPLETE)) { guard(spinlock_irq)(&sdev->ipc_lock); diff --git a/sound/soc/sof/amd/acp.h b/sound/soc/sof/amd/acp.h index 2b7ea8c64106..7bcb76676a98 100644 --- a/sound/soc/sof/amd/acp.h +++ b/sound/soc/sof/amd/acp.h @@ -116,6 +116,8 @@ #define ACP_SRAM_PAGE_COUNT 128 #define ACP6X_SDW_MAX_MANAGER_COUNT 2 #define ACP70_SDW_MAX_MANAGER_COUNT ACP6X_SDW_MAX_MANAGER_COUNT +#define ACP_DSP_MSG_SET 1 +#define ACP_DSP_ACK_SET 1 enum clock_source { ACP_CLOCK_96M = 0, From 25b17c06040fae60518b4ff9c46f2bb12285d538 Mon Sep 17 00:00:00 2001 From: Vijendar Mukunda Date: Tue, 9 Jun 2026 21:38:45 +0530 Subject: [PATCH 3/6] ASoC: SOF: amd: set ipc flags to zero As per design, set IPC conf structure flags to zero during acp init sequence. Link: https://github.com/thesofproject/linux/pull/5642 Signed-off-by: Vijendar Mukunda Tested-by: Umang Jain Link: https://patch.msgid.link/20260609160938.3717513-2-Vijendar.Mukunda@amd.com Signed-off-by: Mark Brown --- sound/soc/sof/amd/acp.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/sound/soc/sof/amd/acp.c b/sound/soc/sof/amd/acp.c index f615b8d1c802..e6af8927baa0 100644 --- a/sound/soc/sof/amd/acp.c +++ b/sound/soc/sof/amd/acp.c @@ -377,6 +377,33 @@ void memcpy_to_scratch(struct snd_sof_dev *sdev, u32 offset, unsigned int *src, snd_sof_dsp_write(sdev, ACP_DSP_BAR, reg_offset + i, src[j]); } +static int acp_init_scratch_mem_ipc_flags(struct snd_sof_dev *sdev) +{ + u32 dsp_msg_write, dsp_ack_write, host_msg_write, host_ack_write; + + dsp_msg_write = sdev->debug_box.offset + + offsetof(struct scratch_ipc_conf, sof_dsp_msg_write); + dsp_ack_write = sdev->debug_box.offset + + offsetof(struct scratch_ipc_conf, sof_dsp_ack_write); + host_msg_write = sdev->debug_box.offset + + offsetof(struct scratch_ipc_conf, sof_host_msg_write); + host_ack_write = sdev->debug_box.offset + + offsetof(struct scratch_ipc_conf, sof_host_ack_write); + /* Initialize host message write flag */ + snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + host_msg_write, 0); + + /* Initialize host ack write flag */ + snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + host_ack_write, 0); + + /* Initialize DSP message write flag */ + snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + dsp_msg_write, 0); + + /* Initialize DSP ack write flag */ + snd_sof_dsp_write(sdev, ACP_DSP_BAR, ACP_SCRATCH_REG_0 + dsp_ack_write, 0); + + return 0; +} + static int acp_memory_init(struct snd_sof_dev *sdev) { struct acp_dev_data *adata = sdev->pdata->hw_pdata; @@ -384,6 +411,7 @@ static int acp_memory_init(struct snd_sof_dev *sdev) snd_sof_dsp_update_bits(sdev, ACP_DSP_BAR, desc->dsp_intr_base + DSP_SW_INTR_CNTL_OFFSET, ACP_DSP_INTR_EN_MASK, ACP_DSP_INTR_EN_MASK); + acp_init_scratch_mem_ipc_flags(sdev); init_dma_descriptor(adata); return 0; From 85f7bf03632bfcdd6cedfb3945b7e387d9487d73 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 10 Jun 2026 10:34:30 +0100 Subject: [PATCH 4/6] ASoC: cs35l56: Fix missing calls to wm_adsp2_remove() Call wm_adsp2_remove() in cs35l56_remove() and the error path of cs35l56_common_probe(). Depends on commit 7d3fb78b5503 ("ASoC: wm_adsp: Fix NULL dereference when removing firmware controls"). The call to wm_halo_init() during driver probe should be paired with a call to wm_adsp2_remove() but this was missing. The consequence would be a memory leak of the control lists in the cs_dsp driver. Fixes: e49611252900 ("ASoC: cs35l56: Add driver for Cirrus Logic CS35L56") Signed-off-by: Richard Fitzgerald Link: https://patch.msgid.link/20260610093432.557375-2-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs35l56.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c index 4fbbdcc87151..3ab5395f15bb 100644 --- a/sound/soc/codecs/cs35l56.c +++ b/sound/soc/codecs/cs35l56.c @@ -1964,11 +1964,14 @@ int cs35l56_common_probe(struct cs35l56_private *cs35l56) cs35l56_dai, ARRAY_SIZE(cs35l56_dai)); if (ret < 0) { dev_err_probe(cs35l56->base.dev, ret, "Register codec failed\n"); - goto err; + goto err_remove_wm_adsp; } return 0; +err_remove_wm_adsp: + wm_adsp2_remove(&cs35l56->dsp); + err: gpiod_set_value_cansleep(cs35l56->base.reset_gpio, 0); regulator_bulk_disable(ARRAY_SIZE(cs35l56->supplies), cs35l56->supplies); @@ -2076,6 +2079,8 @@ void cs35l56_remove(struct cs35l56_private *cs35l56) destroy_workqueue(cs35l56->dsp_wq); + wm_adsp2_remove(&cs35l56->dsp); + pm_runtime_dont_use_autosuspend(cs35l56->base.dev); pm_runtime_suspend(cs35l56->base.dev); pm_runtime_disable(cs35l56->base.dev); From 344a12ca7ba6e10f9779476780afe9d977d47322 Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 10 Jun 2026 10:34:31 +0100 Subject: [PATCH 5/6] ASoC: cs35l56: Prevent double-free of debugfs Invalidate the debugfs pointer after debugfs_remove_recursive() in cs35l56_remove_cal_debugfs(). This prevents a double-free situation when a future commit adds proper failure cleanup in cs35l56_component_probe(). As described by Sashiko (including the future cs35l56_component_probe() cleanup commit): During a normal component unbind, cs35l56_component_remove() calls cs35l56_remove_cal_debugfs() which removes the directory but leaves a dangling pointer. If the component is later bound again, but _cs35l56_component_probe() fails early (for example, if the init_completion times out), this new error path will call cs35l56_component_remove(). This causes cs35l56_remove_cal_debugfs() to be called again with the dangling cs35l56_base->debugfs pointer from the previous lifecycle, resulting in a use-after-free in debugfs_remove_recursive(). Fixes: f7097161e94c ("ASoC: cs35l56: Add common code for factory calibration") Reported-by: sashiko Link: https://sashiko.dev/#/patchset/20260609120738.284770-1-rf%40opensource.cirrus.com Signed-off-by: Richard Fitzgerald Link: https://patch.msgid.link/20260610093432.557375-3-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs35l56-shared.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/soc/codecs/cs35l56-shared.c b/sound/soc/codecs/cs35l56-shared.c index 795e2764d67e..e04b114a32b7 100644 --- a/sound/soc/codecs/cs35l56-shared.c +++ b/sound/soc/codecs/cs35l56-shared.c @@ -1293,6 +1293,7 @@ EXPORT_SYMBOL_NS_GPL(cs35l56_create_cal_debugfs, "SND_SOC_CS35L56_SHARED"); void cs35l56_remove_cal_debugfs(struct cs35l56_base *cs35l56_base) { debugfs_remove_recursive(cs35l56_base->debugfs); + cs35l56_base->debugfs = ERR_PTR(-ENOENT); } EXPORT_SYMBOL_NS_GPL(cs35l56_remove_cal_debugfs, "SND_SOC_CS35L56_SHARED"); From a0df7522dfb098d56b42560247082d6f5a8581dd Mon Sep 17 00:00:00 2001 From: Richard Fitzgerald Date: Wed, 10 Jun 2026 10:34:32 +0100 Subject: [PATCH 6/6] ASoC: cs35l56: Cleanup if component_probe fails If cs35l56_component_probe() fails, call cs35l56_component_remove() to clean up. All the cleanup in cs35l56_component_remove() is the same cleanup that would need to be done (at least partially) if cs35l56_component_probe() fails. So calling cs35l56_component_remove() avoids convoluted cleanup gotos and duplicated code in cs35l56_component_probe(). The only action in cs35l56_component_remove() that is nominally dependent on having completed the component_probe() action is the call to wm_adsp2_component_remove(). Though it is currently safe to call that even if wm_adsp2_component_probe() was not called. However, wm_adsp2_component_probe() has been trivially updated to check itself whether it needs to cleanup. Fixes: e49611252900 ("ASoC: cs35l56: Add driver for Cirrus Logic CS35L56") Signed-off-by: Richard Fitzgerald Link: https://patch.msgid.link/20260610093432.557375-4-rf@opensource.cirrus.com Signed-off-by: Mark Brown --- sound/soc/codecs/cs35l56.c | 13 ++++++++++++- sound/soc/codecs/wm_adsp.c | 7 +++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/cs35l56.c b/sound/soc/codecs/cs35l56.c index 3ab5395f15bb..3641ac1378c7 100644 --- a/sound/soc/codecs/cs35l56.c +++ b/sound/soc/codecs/cs35l56.c @@ -1326,7 +1326,7 @@ VISIBLE_IF_KUNIT int cs35l56_set_fw_name(struct snd_soc_component *component) } EXPORT_SYMBOL_IF_KUNIT(cs35l56_set_fw_name); -static int cs35l56_component_probe(struct snd_soc_component *component) +static int _cs35l56_component_probe(struct snd_soc_component *component) { struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component); struct cs35l56_private *cs35l56 = snd_soc_component_get_drvdata(component); @@ -1426,6 +1426,17 @@ static void cs35l56_component_remove(struct snd_soc_component *component) cs35l56->component = NULL; } +static int cs35l56_component_probe(struct snd_soc_component *component) +{ + int ret; + + ret = _cs35l56_component_probe(component); + if (ret < 0) + cs35l56_component_remove(component); + + return ret; +} + static int cs35l56_set_bias_level(struct snd_soc_component *component, enum snd_soc_bias_level level) { diff --git a/sound/soc/codecs/wm_adsp.c b/sound/soc/codecs/wm_adsp.c index ca630c9948e4..baa75e7ff53b 100644 --- a/sound/soc/codecs/wm_adsp.c +++ b/sound/soc/codecs/wm_adsp.c @@ -1170,7 +1170,14 @@ EXPORT_SYMBOL_GPL(wm_adsp2_component_probe); int wm_adsp2_component_remove(struct wm_adsp *dsp, struct snd_soc_component *component) { + if (!dsp) + return 0; + + if (!dsp->component) + return 0; + cs_dsp_cleanup_debugfs(&dsp->cs_dsp); + dsp->component = NULL; return 0; }