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 <peter.ujfalusi@linux.intel.com>

=== 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 <peter.ujfalusi@linux.intel.com>

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().

Author: ujfalusi

sound/soc/sof/sof-audio.c

@@ -547,20 +547,16 @@ 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 +576,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 +727,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;
 }