Re: [PATCH v3 3/6] ASoC: qcom: common: Add generic headset jack helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cleanup in qcom_snd_wcd_jack_setup error path leads to a Use-After-Free of the jack object.
- [High] Static array `qcom_headset_jack_pins` suffers from list corruption if multiple sound cards are instantiated.
--

commit b92ab42f39f7bdd943113ea9586e913e225e1a7f
Author: Hongyang Zhao <hongyang.zhao@thundersoft.com>

ASoC: qcom: common: Add generic headset jack helpers

This commit factors out common jack allocation and DAPM pin registration
into a private initializer. It adds a generic setup helper to attach jacks
to codec components and a matching cleanup helper. The WCD setup behavior
remains unchanged.

> diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
> index d9f256d519733..83b745f617a86 100644
> --- a/sound/soc/qcom/common.c
> +++ b/sound/soc/qcom/common.c
> @@ -339,13 +339,11 @@ static struct snd_soc_jack_pin qcom_headset_jack_pins[] = {
>  	},
>  };
>  
> -int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd,
> -			    struct snd_soc_jack *jack, bool *jack_setup)
> +static int qcom_snd_headset_jack_init(struct snd_soc_card *card,
> +				      struct snd_soc_jack *jack,
> +				      bool *jack_setup)
>  {
> -	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> -	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> -	struct snd_soc_card *card = rtd->card;
> -	int rval, i;
> +	int rval;
>  
>  	if (!*jack_setup) {
>  		rval = snd_soc_card_jack_new_pins(card, "Headset Jack",

[Severity: High]
This is a pre-existing issue, but can this static array usage cause list
corruption if multiple sound cards are instantiated concurrently?

qcom_headset_jack_pins is a global static array passed into
snd_soc_card_jack_new_pins() here. The ALSA core function
snd_soc_jack_add_pins() directly mutates this array by calling
INIT_LIST_HEAD and list_add on the embedded list nodes of each element.

If multiple sound cards or multiple instances of a machine driver are
instantiated, the !*jack_setup check evaluates to true for the new instance
since the boolean resides in the per-card private data. The second instance
would overwrite the list_head pointers in the shared static array elements,
corrupting the linked lists of the first card and causing list traversal
crashes.

[ ... ]

> +int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd,
> +			    struct snd_soc_jack *jack, bool *jack_setup)
> +{
> +	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
> +	struct snd_soc_dai *codec_dai = snd_soc_rtd_to_codec(rtd, 0);
> +	struct snd_soc_card *card = rtd->card;
> +	int rval, i;
> +
> +	rval = qcom_snd_headset_jack_init(card, jack, jack_setup);
> +	if (rval)
> +		return rval;
> +
>  	switch (cpu_dai->id) {

[Severity: High]
This is a pre-existing issue, but I noticed the new generic helper
qcom_snd_headset_jack_setup() includes a cleanup on failure, whereas
this existing function does not.

In qcom_snd_wcd_jack_setup(), if snd_soc_component_set_jack() fails for a
codec DAI after successfully configuring earlier DAIs in the loop, the
function returns immediately without rolling back the configurations:

	for_each_rtd_codec_dais(rtd, i, codec_dai) {
		rval = snd_soc_component_set_jack(codec_dai->component,
						  jack, NULL);
		if (rval != 0 && rval != -ENOTSUPP) {
			dev_warn(card->dev, "Failed to set jack: %d\n", rval);
			return rval;
		}
	}

A probe failure will cause the sound card and its dynamically allocated
snd_soc_jack structure to be freed.

The successfully configured codec components (which reside in separate
codec drivers and persist beyond the sound card probe failure) will retain
dangling pointers to the freed jack. If a hardware jack event triggers an
interrupt later, the codec driver will attempt to report it using the
dangling pointer, resulting in a use-after-free kernel panic.

Should this function be updated to use a cleanup mechanism similar to what
was added for the new generic setup helper?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-rubikpi-next-20260605-v3-0-f49146d85af3@thundersoft.com?part=3