Re: [PATCH 4/4] ASoC: meson: aiu: use aiu-formatter-i2s to format I2S output data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Modifying a global static DAPM widget template with a devm-managed pointer creates a Use-After-Free vulnerability.
- [High] Initialization order flaw between `snd_soc_register_component()` and `gx_formatter_create()` causes a type confusion kernel panic.
--

commit a12ba200fe5e3358b227094679f8a17e9ac53591
Author: Valerio Setti <vsetti@baylibre.com>

ASoC: meson: aiu: use aiu-formatter-i2s to format I2S output data

This commit introduces a new DAPM widget for the I2S formatter and routes
it between the FIFO and output DAI interface. It also removes the existing
I2S output formatting code from aiu-encoder-i2s.

> diff --git a/sound/soc/meson/aiu.c b/sound/soc/meson/aiu.c
> index f2890111c1d2c..b09c2058eacaf 100644
> --- a/sound/soc/meson/aiu.c
> +++ b/sound/soc/meson/aiu.c

[ ... ]

> @@ -29,13 +29,22 @@ static SOC_ENUM_SINGLE_DECL(aiu_spdif_encode_sel_enum, AIU_I2S_MISC,
>  static const struct snd_kcontrol_new aiu_spdif_encode_mux =
>  	SOC_DAPM_ENUM("SPDIF Buffer Src", aiu_spdif_encode_sel_enum);
>  
> -static const struct snd_soc_dapm_widget aiu_cpu_dapm_widgets[] = {
> -	SND_SOC_DAPM_MUX("SPDIF SRC SEL", SND_SOC_NOPM, 0, 0,
> -			 &aiu_spdif_encode_mux),
> +#define AIU_WIDGET_SPDIF_SRC_SEL	0
> +#define AIU_WIDGET_I2S_FORMATTER	1
> +
> +static struct snd_soc_dapm_widget aiu_cpu_dapm_widgets[] = {

Is it safe to modify this global static template with a devm-managed
pointer later in aiu_probe?

If a subsequent initialization step fails and aiu_probe returns an error,
the devres subsystem will automatically free the allocated memory. However,
the dangling pointer will remain in this global static template.

Upon a subsequent probe deferral, will snd_soc_register_component clone
this corrupted template, bringing the freed pointer into the active DAPM
context and causing a use-after-free when gx_formatter_event dereferences
w->priv?

[ ... ]

> @@ -291,6 +305,14 @@ static int aiu_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* Allocate the aiu-formatter into its widget */
> +	ret = gx_formatter_create(dev, &aiu_cpu_dapm_widgets[AIU_WIDGET_I2S_FORMATTER],
> +				  &aiu_formatter_i2s_drv, map);

Could this initialization order cause a type confusion panic?
snd_soc_register_component is invoked right before this in aiu_probe.

If ASoC triggers synchronous card binding upon component registration, the
DAPM widgets are cloned from the template while w->priv is still NULL.

Later, during audio playback, if gx_formatter_event runs, it checks
w->priv, finds it NULL, and executes the fallback path using
snd_soc_component_get_drvdata.

The component drvdata for the AIU CPU component is explicitly set to
struct aiu * via platform_set_drvdata earlier in the probe, not a
struct gx_formatter *.

Will the code erroneously treat the struct aiu * as a struct gx_formatter *
and attempt to dereference its fields, resulting in a panic?

> +	if (ret) {
> +		dev_err(dev, "Failed to allocate aiu formatter\n");
> +		goto err;
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-reshape-aiu-as-axg-v1-0-53b457784ff3@baylibre.com?part=4

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic