From cb65cb1cc76fceb400ba86281442025edcd50eb2 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 11:17:58 +0300 Subject: [PATCH 1/6] fixup! ASoC: SOF: sof-audio: harden recursive widget free walk Revert the sink-path traversal changes, keeping only the NULL spipe / pipe_widget checks in sof_widget_free_unlocked(). path->walking is DFS stack state: it means "this edge is currently being walked", and it has to be unwound by whoever set it. Not clearing it after the recursive call makes it a permanent 'visited' mark, so the FREE walk traverses a different set of edges than the SETUP walk, which unbalances swidget->use_count: - sof_walk_widgets_in_order() starts a walk from every aif_in (playback) or every dai_out/output (capture) widget in the list. With aggregated DAIs the branches reconverge on the shared widgets towards the FE. SETUP walks and increments them once per branch, but on FREE the first branch leaves the shared edges marked and the second branch skips them entirely, so use_count never reaches 0. The widget_free IPC is then never sent, the cores are never put and spipe->complete is never cleared: the DSP side leaks and the next stream reuses a widget that was never re-configured. - The same happens within a single walk whenever two branches of the list reconverge, as the shared edge below the merge point must be walked once per incoming branch. Clearing the flags after the loop cannot fix either case, and doing it for both directions of every widget in the list is unsafe on its own: walking is owned by the walk in progress, and the free walk does not hold the DAPM mutex, so a concurrent dapm_power_widgets() may have those same edges on its stack. The rest of the traversal churn is not needed either. FREE only sends IPC, it never removes DAPM paths, so the _safe iterator has nothing to protect against here - and against a concurrent topology teardown it would not help, as next_p is read before the recursion. p->sink cannot be NULL for a linked path, and the list can never be NULL as sof_widget_list_free() already checks it. Signed-off-by: Peter Ujfalusi === everything below is dropped by 'git rebase --autosquash' === Once squashed, the only thing left of the original commit are the three hunks in sof_widget_free_unlocked(); the traversal is byte identical to the pre-2712ab5d30d6 state. The original subject no longer describes the patch, so use this message for the squashed commit: ASoC: SOF: sof-audio: do not dereference swidget->spipe unconditionally on free sof_widget_free_unlocked() dereferences swidget->spipe without checking it for two things: swidget->spipe->complete for a scheduler widget, and swidget->spipe->pipe_widget for the recursive free of the pipeline's scheduler widget. Both can be reached with spipe or pipe_widget not set, which oopses in the free path - where there is nothing left to bail out to. Check both before use and cache swidget->spipe in the local spipe variable that is already there. A widget with no pipeline has nothing to put or complete, and no scheduler widget to free, so skipping is the correct behaviour. No functional change for a widget that was successfully set up: sof_widget_setup_unlocked() already rejects a dynamic pipeline widget with no spipe or no spipe->pipe_widget with -EINVAL. Signed-off-by: Peter Ujfalusi Two things to resolve before sending it out: - The oops was most likely hit via the compress free path (ipc4-compress.c sof_widget_list_free()), but the details were not recovered. Replace "can be reached with spipe or pipe_widget not set" with the actual path and add a Fixes: tag once known. - If it cannot be reproduced, drop the patch instead: as the last paragraph shows, the guards are unreachable for any widget that went through sof_widget_setup_unlocked(). --- sound/soc/sof/sof-audio.c | 52 +++++---------------------------------- 1 file changed, 6 insertions(+), 46 deletions(-) diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 0e4e1bb8c35ff1..0b2f41f4a59bfa 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -547,20 +547,15 @@ sof_prepare_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget * free all widgets in the sink path starting from the source widget * (DAI type for capture, AIF type for playback) */ -static int sof_free_widgets_in_path_internal(struct snd_sof_dev *sdev, - struct snd_soc_dapm_widget *widget, - int dir, struct snd_sof_pcm *spcm, - struct snd_soc_dapm_widget_list *list) +static int sof_free_widgets_in_path(struct snd_sof_dev *sdev, struct snd_soc_dapm_widget *widget, + int dir, struct snd_sof_pcm *spcm) { + struct snd_soc_dapm_widget_list *list = spcm->stream[dir].list; struct snd_sof_widget *swidget = widget->dobj.private; struct snd_soc_dapm_path *p; - struct snd_soc_dapm_path *next_p; int err; int ret = 0; - if (!list) - return 0; - if (is_virtual_widget(sdev, widget, __func__)) return 0; @@ -580,52 +575,23 @@ static int sof_free_widgets_in_path_internal(struct snd_sof_dev *sdev, ret = err; sink_free: /* free all widgets in the sink paths even in case of error to keep use counts balanced */ - snd_soc_dapm_widget_for_each_path_safe(widget, SND_SOC_DAPM_DIR_IN, p, next_p) { + snd_soc_dapm_widget_for_each_sink_path(widget, p) { if (!p->walking) { - if (!p->sink) - continue; - if (!widget_in_list(list, p->sink)) continue; p->walking = true; - err = sof_free_widgets_in_path_internal(sdev, p->sink, - dir, spcm, list); + err = sof_free_widgets_in_path(sdev, p->sink, dir, spcm); if (err < 0) ret = err; + p->walking = false; } } return ret; } -static int sof_free_widgets_in_path(struct snd_sof_dev *sdev, - struct snd_soc_dapm_widget *widget, - int dir, struct snd_sof_pcm *spcm) -{ - return sof_free_widgets_in_path_internal(sdev, widget, dir, spcm, - spcm->stream[dir].list); -} - -static void sof_reset_path_walking_flags(struct snd_soc_dapm_widget_list *list) -{ - struct snd_soc_dapm_widget *widget; - struct snd_soc_dapm_path *p; - int i; - - if (!list) - return; - - for_each_dapm_widgets(list, i, widget) { - snd_soc_dapm_widget_for_each_sink_path(widget, p) - p->walking = false; - - snd_soc_dapm_widget_for_each_source_path(widget, p) - p->walking = false; - } -} - /* * set up all widgets in the sink path starting from the source widget * (DAI type for capture, AIF type for playback). @@ -760,17 +726,11 @@ sof_walk_widgets_in_order(struct snd_sof_dev *sdev, struct snd_sof_pcm *spcm, return -EINVAL; } if (ret < 0) { - if (op == SOF_WIDGET_FREE) - sof_reset_path_walking_flags(list); - dev_err(sdev->dev, "Failed to %s connected widgets\n", str); return ret; } } - if (op == SOF_WIDGET_FREE) - sof_reset_path_walking_flags(list); - return 0; } From 64a796fca7aa72808536ae8fd16f68196c873292 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 14:52:26 +0300 Subject: [PATCH 2/6] fixup! ASoC: SOF: ipc4-topology: Support init_ext_module_data for process modules sof_ipc4_add_init_ext_module_data() writes the object header and copies init_ext_module_size bytes into the payload buffer before anything has checked that they fit. The only bounds check is in the caller, after all objects have been added, so an oversized module data would have already overflowed the kzalloc(max_payload_size) buffer by the time it runs. The other object in the array is of a fixed, known size and always fits, but the module data is provided by the module which is being set up, so validate it in the helper before any of it is written and let the caller propagate the error. Suggested commit message for the squashed patch: ASoC: SOF: ipc4-topology: Support init_ext_module_data for process modules Add support for handling init_ext_module_data for process modules, which is going to be used by decoder and encoder type of process modules. The support is generic and it can be extended to other type of process modules or other module types than process with a small update of sof_ipc4_add_init_ext_module_data() function. The module data is of variable size, unlike the other objects of the array, so its size is validated against the maximum payload size before it is copied into the message. Signed-off-by: Peter Ujfalusi Reviewed-by: Liam Girdwood Signed-off-by: Peter Ujfalusi (cherry picked from commit 572833d3b99ea7550ea198cf2ec7b564486578ce) --- sound/soc/sof/ipc4-topology.c | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index a94f4baec632a5..092287a4e1f14a 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -3296,7 +3296,7 @@ static void sof_ipc4_add_init_ext_dp_memory_data(struct snd_sof_dev *sdev, *ext_pos += DIV_ROUND_UP(sizeof(*dp_mem_data), sizeof(u32)); } -static void +static int sof_ipc4_add_init_ext_module_data(struct snd_sof_dev *sdev, struct sof_ipc4_process *process, u32 *payload, u32 *ext_pos, @@ -3304,6 +3304,22 @@ sof_ipc4_add_init_ext_module_data(struct snd_sof_dev *sdev, { u32 data_size = process->init_ext_module_size; void *data = process->init_ext_module_data; + size_t needed; + + /* + * Unlike the other objects, the module data is of variable size, + * provided by the module which is being set up. + * Make sure that the object fits into the payload buffer before any of + * it is written. + */ + needed = ((size_t)*ext_pos + DIV_ROUND_UP(sizeof(**hdr), sizeof(u32)) + + DIV_ROUND_UP(data_size, sizeof(u32))) * sizeof(u32); + if (needed > sdev->ipc->max_payload_size) { + dev_err(sdev->dev, + "Max ipc payload size %zu exceeded by module data: %zu\n", + sdev->ipc->max_payload_size, needed); + return -EINVAL; + } *hdr = (struct sof_ipc4_module_init_ext_object *)&payload[*ext_pos]; (*hdr)->header = SOF_IPC4_MOD_INIT_EXT_OBJ_ID(SOF_IPC4_MOD_INIT_DATA_ID_MODULE_DATA) | @@ -3313,6 +3329,8 @@ sof_ipc4_add_init_ext_module_data(struct snd_sof_dev *sdev, memcpy(&payload[*ext_pos], data, data_size); *ext_pos += DIV_ROUND_UP(data_size, sizeof(u32)); + + return 0; } static int sof_ipc4_widget_mod_init_msg_payload(struct snd_sof_dev *sdev, @@ -3329,6 +3347,7 @@ static int sof_ipc4_widget_mod_init_msg_payload(struct snd_sof_dev *sdev, int new_size; u32 *payload; u32 ext_pos; + int ret; if (!in_dp_domain && !has_ext_data) return 0; @@ -3347,9 +3366,14 @@ static int sof_ipc4_widget_mod_init_msg_payload(struct snd_sof_dev *sdev, sof_ipc4_add_init_ext_dp_memory_data(sdev, swidget, payload, &ext_pos, &hdr); - if (has_ext_data) - sof_ipc4_add_init_ext_module_data(sdev, process, payload, - &ext_pos, &hdr); + if (has_ext_data) { + ret = sof_ipc4_add_init_ext_module_data(sdev, process, payload, + &ext_pos, &hdr); + if (ret) { + kfree(payload); + return ret; + } + } /* Set last bit for the last object in the array */ hdr->header |= SOF_IPC4_MOD_INIT_EXT_OBJ_LAST_MASK; From 8d835d3f5192c10132ffe78b3cf54a11d8c548de Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 17:18:28 +0300 Subject: [PATCH 3/6] Revert "ASoC: soc-pcm: Allocate be_substream->runtime for compressed streams" This reverts commit 89a72449616b2a2f149e93d02e49c39d3311f4da. Signed-off-by: Peter Ujfalusi --- sound/soc/soc-pcm.c | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index ae6aa2b1aa4653..0e49290a8c903b 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -1790,8 +1790,6 @@ void dpcm_be_dai_stop(struct snd_soc_pcm_runtime *fe, int stream, } __soc_pcm_close(be, be_substream); - if (fe->fe_compr) - kfree(be_substream->runtime); be_substream->runtime = NULL; be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; } @@ -1839,26 +1837,18 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) dev_dbg(be->dev, "ASoC: open %s BE %s\n", snd_pcm_direction_name(stream), be->dai_link->name); - if (!fe->fe_compr) { - be_substream->runtime = fe_substream->runtime; - } else { - be_substream->runtime = kzalloc(sizeof(*be_substream->runtime), - GFP_KERNEL); - if (!be_substream->runtime) { - err = -ENOMEM; - goto unwind; - } - } - + be_substream->runtime = fe_substream->runtime; err = __soc_pcm_open(be, be_substream); if (err < 0) { - if (fe->fe_compr) { - kfree(be_substream->runtime); - be_substream->runtime = NULL; - } + be->dpcm[stream].users--; + if (be->dpcm[stream].users < 0) + dev_err(be->dev, "ASoC: no users %s at unwind %s\n", + snd_pcm_direction_name(stream), + dpcm_state_string(be->dpcm[stream].state)); + + be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; goto unwind; } - be->dpcm[stream].be_start = 0; be->dpcm[stream].state = SND_SOC_DPCM_STATE_OPEN; count++; @@ -1867,14 +1857,6 @@ int dpcm_be_dai_startup(struct snd_soc_pcm_runtime *fe, int stream) return count; unwind: - be->dpcm[stream].users--; - if (be->dpcm[stream].users < 0) - dev_err(be->dev, "ASoC: no users %s at unwind %s\n", - snd_pcm_direction_name(stream), - dpcm_state_string(be->dpcm[stream].state)); - - be->dpcm[stream].state = SND_SOC_DPCM_STATE_CLOSE; - dpcm_be_dai_startup_rollback(fe, stream, dpcm); return soc_pcm_ret(fe, err); From 3cc7fe9ab156eeb73466a32c9b416dca01f45bf5 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 15:40:08 +0300 Subject: [PATCH 4/6] ASoC: soc-compress: Provide a runtime for the compressed FE substream When a compressed stream is used on the FE side of a DPCM link, the BE is still running as 'normal' PCM. DPCM lends the runtime of the FE to every BE it opens and dpcm_be_reparent() re-points it at another FE when the lending one disconnects. The internal PCM created for a compressed FE by snd_soc_new_compress() is never opened via the PCM API, so its substream has no runtime and the BEs of a compressed FE were left with a NULL one. BE DAI and codec drivers can look at substream->runtime, they add their constraints to it in startup(), for example cs42l43, cs42l42, hdac_hdmi. The other users of compressed DPCM do not hit this as their BEs are DSP internal ports, driven by code which takes everything from the hw_params it is passed and never looks at substream->runtime. The BEs here end at generic CODEC drivers which are shared with the non DPCM case and cannot be expected to know about the FE type. Allocate a runtime for the compressed FE substream while the stream is open and fill it in from the BE parameters once the mandatory machine level be_hw_params_fixup() has run, so that the compressed FE lends a valid and populated runtime just like a PCM FE does. This also allows a BE to be shared between a compressed and a PCM FE, which is needed to play a notification over PCM to an endpoint which a compressed stream is already using. Signed-off-by: Peter Ujfalusi --- sound/soc/soc-compress.c | 90 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 3a8da272248328..46986220d4ced1 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -130,6 +131,85 @@ static int soc_compr_open(struct snd_compr_stream *cstream) return ret; } +/* + * The internal PCM of a compressed FE is never opened via the PCM API, so its + * substream has no runtime attached to it. + * + * DPCM lends the runtime of the FE to every BE it opens and re-points it at + * another FE when the lending one goes away, so the compressed FE must provide + * one as well. Without it the BEs are left with a NULL runtime, which oopses + * in BE DAI and CODEC drivers looking at substream->runtime, and a BE can not + * be shared between a compressed and a PCM FE at all. + * + * The runtime is owned by the FE for as long as the compressed stream is open. + */ +static int soc_compr_alloc_fe_runtime(struct snd_soc_pcm_runtime *fe, int stream) +{ + struct snd_pcm_substream *fe_substream = snd_soc_dpcm_get_substream(fe, stream); + + if (!fe_substream || fe_substream->runtime) + return 0; + + fe_substream->runtime = kzalloc_obj(*fe_substream->runtime); + if (!fe_substream->runtime) + return -ENOMEM; + + return 0; +} + +static void soc_compr_free_fe_runtime(struct snd_soc_pcm_runtime *fe, int stream) +{ + struct snd_pcm_substream *fe_substream = snd_soc_dpcm_get_substream(fe, stream); + + if (!fe_substream || !fe_substream->runtime) + return; + + /* BE startup callbacks may have added hw constraint rules */ + kfree(fe_substream->runtime->hw_constraints.rules); + kfree(fe_substream->runtime); + fe_substream->runtime = NULL; +} + +/* + * The BE parameters of a compressed FE are set up by the machine level + * be_hw_params_fixup(), which is mandatory for a compressed BE, see + * soc_compr_set_params_fe(). Once they are fixed up, use them to fill in the + * runtime the BEs have been lent, so that a BE DAI or CODEC driver sees the + * format it is being configured for. + */ +static void soc_compr_set_fe_runtime(struct snd_soc_pcm_runtime *fe, int stream) +{ + struct snd_pcm_substream *fe_substream = snd_soc_dpcm_get_substream(fe, stream); + struct snd_pcm_runtime *runtime; + struct snd_soc_dpcm *dpcm; + + snd_soc_dpcm_mutex_assert_held(fe); + + if (!fe_substream || !fe_substream->runtime) + return; + + runtime = fe_substream->runtime; + + for_each_dpcm_be(fe, stream, dpcm) { + struct snd_pcm_hw_params *params = &dpcm->be->dpcm[stream].hw_params; + int bits = snd_pcm_format_physical_width(params_format(params)); + + /* skip a BE which has not been fixed up */ + if (bits <= 0) + continue; + + runtime->access = params_access(params); + runtime->format = params_format(params); + runtime->subformat = params_subformat(params); + runtime->channels = params_channels(params); + runtime->rate = params_rate(params); + + runtime->sample_bits = bits; + runtime->frame_bits = bits * runtime->channels; + break; + } +} + static int soc_compr_open_fe(struct snd_compr_stream *cstream) { struct snd_soc_pcm_runtime *fe = cstream->private_data; @@ -141,6 +221,10 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) snd_soc_card_mutex_lock(fe->card); + ret = soc_compr_alloc_fe_runtime(fe, stream); + if (ret < 0) + goto be_err; + ret = dpcm_path_get(fe, stream, &list); if (ret < 0) goto be_err; @@ -195,6 +279,7 @@ static int soc_compr_open_fe(struct snd_compr_stream *cstream) dpcm_path_put(&list); snd_soc_dpcm_mutex_unlock(fe); be_err: + soc_compr_free_fe_runtime(fe, stream); fe->dpcm[stream].runtime_update = SND_SOC_DPCM_UPDATE_NO; snd_soc_card_mutex_unlock(fe->card); return ret; @@ -247,6 +332,9 @@ static int soc_compr_free_fe(struct snd_compr_stream *cstream) snd_soc_dai_compr_shutdown(cpu_dai, cstream, 0); + /* all BEs are shut down and disconnected, the runtime is unused now */ + soc_compr_free_fe_runtime(fe, stream); + snd_soc_card_mutex_unlock(fe->card); return 0; } @@ -450,6 +538,8 @@ static int soc_compr_set_params_fe(struct snd_compr_stream *cstream, snd_soc_dpcm_mutex_lock(fe); ret = dpcm_be_dai_hw_params(fe, stream); + if (!ret) + soc_compr_set_fe_runtime(fe, stream); snd_soc_dpcm_mutex_unlock(fe); if (ret < 0) goto out; From e050c9149d73b7740a9197e0f019b33c0868d5c4 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 15:11:51 +0300 Subject: [PATCH 5/6] fixup! ASoC: SOF: Add support for IPC4 compressed sof_ipc4_compr_drain_done() checks swidget->spipe before walking the widget list, but the error path taken when no host copier is found on the pipeline goes on to dereference swidget->spipe->pipe_widget to print the scheduler widget name, without checking pipe_widget itself. The function runs from the IPC RX path, so an unset pipe_widget oopses in interrupt context. Check it together with spipe, the same way sof_widget_free_unlocked() does. Suggested commit message for the squashed patch: no change needed, the existing one still describes the patch correctly. Signed-off-by: Peter Ujfalusi (cherry picked from commit 6987c5022032eff92b93e63891271d8d0468f962) --- sound/soc/sof/ipc4-compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/sof/ipc4-compress.c b/sound/soc/sof/ipc4-compress.c index 05cebb780032e7..493067fc8eddda 100644 --- a/sound/soc/sof/ipc4-compress.c +++ b/sound/soc/sof/ipc4-compress.c @@ -735,7 +735,7 @@ void sof_ipc4_compr_drain_done(struct snd_sof_dev *sdev, void *ipc_message) return; } - if (!swidget->spipe) + if (!swidget->spipe || !swidget->spipe->pipe_widget) return; /* Find the swidget of the host copier on the same pipeline */ From 33c033468adb0f375ad99c81cd40b0d51e195a1a Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Mon, 24 Aug 2026 16:41:07 +0300 Subject: [PATCH 6/6] fixup! ASoC: SOF: Add support for IPC4 compressed sof_ipc4_compr_set_params() frees the list of DAPM widgets on its error paths but never unprepares them, so the widgets are left prepared with their prepare time resources still taken. sof_ipc4_compr_free() can not clean up after it either: it does call sof_widget_list_unprepare(), but by then spcm->stream[dir].list is NULL and there is no list left to walk. The widgets are prepared by sof_pcm_setup_connected_widgets(), so every error path taken after it is affected, a failing snd_sof_boot_dsp_firmware() for example. Use sof_widget_list_unprepare(), which walks the widgets before freeing the list and clearing spcm->stream[dir].list, the exact sequence which is open coded here otherwise. It is also safe on the path where sof_widget_list_setup() has failed and unprepared the widgets already, since sof_unprepare_widgets_in_path() skips widgets which are not prepared. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/ipc4-compress.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sound/soc/sof/ipc4-compress.c b/sound/soc/sof/ipc4-compress.c index 493067fc8eddda..ffa987143af4aa 100644 --- a/sound/soc/sof/ipc4-compress.c +++ b/sound/soc/sof/ipc4-compress.c @@ -323,7 +323,6 @@ static int sof_ipc4_compr_set_params(struct snd_soc_component *component, struct snd_sof_platform_stream_params *platform_params; struct sof_ipc4_timestamp_info *time_info; struct snd_compr_params *compr_params; - struct snd_soc_dapm_widget_list *list; struct snd_sof_widget *host_swidget; struct sof_ipc4_process *process; struct snd_pcm_hw_params p = {0}; @@ -497,9 +496,7 @@ static int sof_ipc4_compr_set_params(struct snd_soc_component *component, process->init_ext_module_size = 0; free_list: - list = spcm->stream[dir].list; - spcm->stream[dir].list = NULL; - snd_soc_dapm_dai_free_widgets(&list); + sof_widget_list_unprepare(sdev, spcm, dir); free_pages: snd_compr_free_pages(cstream);