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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] sound/soc: NULL pointer dereference in tegra_wm8994_remove() during unload
- [Medium] ASoC: tegra: dropped headphone and button events due to SND_JACK_MICROPHONE
--

commit c25c1a719a0690a201ec3decc5bc7bc2cec57601
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..1550faeba7256
> --- /dev/null
> +++ b/sound/soc/tegra/tegra_wm8994.c
> @@ -0,0 +1,175 @@

[ ... ]

> +static int tegra_wm8994_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct tegra_machine *machine = snd_soc_card_get_drvdata(rtd->card);
> +	int err;
> +
> +	err = tegra_asoc_machine_init(rtd);
> +	if (err)
> +		return err;
> +
> +	if (!machine->gpiod_mic_det && machine->asoc->add_mic_jack) {
> +		struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> +		struct snd_soc_component *component = codec_dai->component;
> +
> +		err = snd_soc_card_jack_new_pins(rtd->card, "Mic Jack",
> +						 SND_JACK_MICROPHONE,
> +						 machine->mic_jack,
> +						 tegra_wm8994_mic_jack_pins,
> +						 ARRAY_SIZE(tegra_wm8994_mic_jack_pins));
> +		if (err) {

[Severity: Medium]
Will this silently drop headphone and button events from the codec?

The ALSA jack is explicitly initialized with only the SND_JACK_MICROPHONE
capability. However, the WM8994 codec handles full headset detection and tries
to report events as SND_JACK_HEADSET | SND_JACK_BTN_0.

Because the ALSA jack type is restricted to SND_JACK_MICROPHONE, the ALSA core
filters out the SND_JACK_HEADPHONE and button bits in snd_jack_report(),
preventing them from reaching the input layer. Additionally, since the DAPM
pin configuration only maps the microphone, automatic DAPM routing will fail
to turn on the Headphone Jack widget upon headset insertion.

> +			dev_err(rtd->dev, "Mic Jack creation failed: %d\n", err);
> +			return err;
> +		}
> +
> +		wm8994_mic_detect(component, machine->mic_jack, 1);
> +	}
> +
> +	return 0;
> +}
> +
> +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 tegra_machine *machine = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);

[Severity: High]
Does this code trigger a NULL pointer dereference during module unload?

When the driver is unbound, devm_snd_soc_register_card() invokes
snd_soc_unregister_card(). This calls soc_cleanup_card_resources(), which
explicitly deletes and frees all PCM runtimes (rtd) from the rtd_list via
snd_soc_remove_pcm_runtime() before calling snd_soc_card_remove(card).

When snd_soc_card_remove() finally invokes this tegra_wm8994_remove() callback,
the call to snd_soc_get_pcm_runtime() will return NULL because the rtd_list
is empty. The code then unconditionally dereferences this NULL pointer here in
rtd->card and the snd_soc_rtd_to_codec(rtd, 0) macro.

> +	struct snd_soc_component *component = codec_dai->component;
> +
> +	if (!machine->gpiod_mic_det && machine->asoc->add_mic_jack)
> +		wm8994_mic_detect(component, NULL, 1);
> +
> +	return 0;
> +}

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