Re: [PATCH v1 2/2] ASoC: tegra: Support WM8994 by machine driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] asoc: tegra: UAF and state unbalance due to invalid micbias in wm8994_mic_detect() teardown
- [High] asoc: tegra: missing .owner = THIS_MODULE leads to module unload crash
- [Low] asoc: tegra: DAPM pin switches reference non-existent widgets
--

commit f9aa3ff5a04d8149180c0f26beabf248ebcd5577
Author: Svyatoslav Ryhel <clamor95@gmail.com>

ASoC: tegra: Support WM8994 by machine driver
    
Add Wolfson Microelectronics WM8994 codec support to the Tegra ASoC
machine driver. The WM8994 codec is found on devices like LG Optimus 2X
(P990/star) and Samsung Captivate Glide (SGH-i927).

> diff --git a/sound/soc/tegra/tegra_wm8994.c b/sound/soc/tegra/tegra_wm8994.c
> new file mode 100644
> index 0000000000000..950fa6ad388f9
> --- /dev/null
> +++ b/sound/soc/tegra/tegra_wm8994.c
[ ... ]
> +static const struct snd_kcontrol_new tegra_wm8994_controls[] = {
> +	SOC_DAPM_PIN_SWITCH("Int Spk"),
> +	SOC_DAPM_PIN_SWITCH("Internal Mic 1"),
> +	SOC_DAPM_PIN_SWITCH("Internal Mic 2"),
> +	SOC_DAPM_PIN_SWITCH("Mic Jack"),
> +	SOC_DAPM_PIN_SWITCH("Line Out"),
> +	SOC_DAPM_PIN_SWITCH("FM Radio"),
> +};

[Severity: Low]
Are there matching DAPM widgets defined for the "Line Out" and "FM Radio"
pin switches?

Neither tegra_asoc_machine.c nor the WM8994 codec driver appears to define
DAPM widgets with these exact names. Referencing non-existent widgets
creates unattached controls that fail to operate correctly when toggled
from userspace.

[ ... ]
> +static int tegra_wm8994_remove(struct snd_soc_card *card)
> +{
> +	struct snd_soc_dai_link *link = &card->dai_link[0];
> +	struct snd_soc_pcm_runtime *rtd = snd_soc_get_pcm_runtime(card, link);
> +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> +	struct snd_soc_component *component = codec_dai->component;
> +
> +	wm8994_mic_detect(component, NULL, 0);

[Severity: High]
Does calling wm8994_mic_detect() with 0 for the micbias argument skip
cleanup and return an error?

In sound/soc/codecs/wm8994.c:wm8994_mic_detect(), when micbias is 0,
it falls into the default switch case, logs a warning, and returns -EINVAL:

	default:
		dev_warn(component->dev, "Invalid MICBIAS %d\n", micbias);
		pm_runtime_put(component->dev);
		return -EINVAL;

Because it returns early, the teardown logic is never executed, and
wm8994->micdet[0].jack is not set to NULL.

When the machine driver is unbound, the snd_jack is freed. However, the
codec still holds the dangling pointer. Can a subsequent microphone IRQ fire,
causing wm8994_mic_irq() to call snd_soc_jack_report() on the freed jack
and leading to a use-after-free crash?

Additionally, is it intentional to call this unconditionally, even if it
was never enabled (for example, when machine->gpiod_mic_det is present)?
This might lead to unbalanced teardown.

> +
> +	return 0;
> +}
[ ... ]
> +static struct snd_soc_card snd_soc_tegra_wm8994 = {
> +	.components = "codec:wm8994",
> +	.dai_link = &tegra_wm8994_dai,
> +	.num_links = 1,
> +	.controls = tegra_wm8994_controls,
> +	.num_controls = ARRAY_SIZE(tegra_wm8994_controls),
> +	.remove = tegra_wm8994_remove,
> +	.fully_routed = true,
> +};

[Severity: High]
Is the .owner field intentionally omitted from this structure?

The driver uses a shared probe function, tegra_asoc_machine_probe(),
from tegra_asoc_machine.c, which contains the fallback:

	if (!card->owner)
		card->owner = THIS_MODULE;

Because THIS_MODULE in the fallback resolves to the snd-soc-tegra-machine
module, the snd-soc-tegra-wm8994 module's reference count is never
incremented when a PCM stream is opened.

Could this allow the snd-soc-tegra-wm8994 module to be unloaded via rmmod
while audio is actively playing, causing any subsequent ALSA core access to
callbacks or data structures in the unloaded module to trigger a kernel oops?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013183226.513658-1-clamor95@gmail.com?part=2