Re: [PATCH v2 09/24] ASoC: SOF: sof-audio: do not dereference swidget->spipe unconditionally on free

[Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>]

On 9/9/26 11:09, Peter Ujfalusi wrote:
> 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>
> Reviewed-by: Liam Girdwood <liam.r.girdwood@intel.com>
> ---
>  sound/soc/sof/sof-audio.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

same, this doesn't seem directly related to compresss offload. Maybe
it's required because of new transitions?

I'd move this earlier or submit as a cleanup separately to reduce the
volume of reviews (24 patches!)

> diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
> index d244e90a734b..0b2f41f4a59b 100644
> --- a/sound/soc/sof/sof-audio.c
> +++ b/sound/soc/sof/sof-audio.c
> @@ -103,7 +103,7 @@ static int sof_widget_free_unlocked(struct snd_sof_dev *sdev,
>  	 * decrement ref count for cores associated with all modules in the pipeline and clear
>  	 * the complete flag
>  	 */
> -	if (swidget->id == snd_soc_dapm_scheduler) {
> +	if (swidget->id == snd_soc_dapm_scheduler && spipe) {
>  		int i;
>  
>  		for_each_set_bit(i, &spipe->core_mask, sdev->num_cores) {
> @@ -115,16 +115,16 @@ static int sof_widget_free_unlocked(struct snd_sof_dev *sdev,
>  					err = ret;
>  			}
>  		}
> -		swidget->spipe->complete = 0;
> +		spipe->complete = 0;
>  	}
>  
>  	/*
>  	 * free the scheduler widget (same as pipe_widget) associated with the current swidget.
>  	 * skip for static pipelines
>  	 */
> -	if (swidget->spipe && swidget->dynamic_pipeline_widget &&
> +	if (spipe && spipe->pipe_widget && swidget->dynamic_pipeline_widget &&
>  	    swidget->id != snd_soc_dapm_scheduler) {
> -		ret = sof_widget_free_unlocked(sdev, swidget->spipe->pipe_widget);
> +		ret = sof_widget_free_unlocked(sdev, spipe->pipe_widget);
>  		if (ret < 0 && !err)
>  			err = ret;
>  	}