linux-kernel - Re: [PATCH v2] ASoC: intel: avs: Fix potential memory leak in avs_pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <24e5abed-9084-4f3d-b620-e272164f687e@intel.com>
Date: Thu, 13 Nov 2025 14:11:19 +0100
From: Cezary Rojewski <cezary.rojewski@...el.com>
To: Abdun Nihaal <nihaal@....iitm.ac.in>
CC: <liam.r.girdwood@...ux.intel.com>, <peter.ujfalusi@...ux.intel.com>,
	<yung-chuan.liao@...ux.intel.com>, <ranjani.sridharan@...ux.intel.com>,
	<kai.vehmanen@...ux.intel.com>, <pierre-louis.bossart@...ux.dev>,
	<broonie@...nel.org>, <perex@...ex.cz>, <tiwai@...e.com>,
	<linux-sound@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] ASoC: intel: avs: Fix potential memory leak in
 avs_pci_probe()

On 2025-11-13 1:04 PM, Abdun Nihaal wrote:
> The link resources allocated in snd_hdac_ext_bus_get_ml_capabilities()
> are not freed on subsequent error paths in avs_pci_probe().
> 
> Fixes: 1affc44ea5dd ("ASoC: Intel: avs: PCI driver implementation")
> Signed-off-by: Abdun Nihaal <nihaal@....iitm.ac.in>

Reviewed-by: Cezary Rojewski <cezary.rojewski@...el.com>

> ---
> 
> v1->v2:
> - Shorten commit message
> - Handle the case when some of the links are allocated, as pointed out
>    by Cezary Rojewski
> 
> Link to V1:
> https://lore.kernel.org/all/20251113104121.79484-1-nihaal@cse.iitm.ac.in/T/#u
> 
>   sound/soc/intel/avs/core.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
> index 6e0e65584c7f..f0d77f3f3a28 100644
> --- a/sound/soc/intel/avs/core.c
> +++ b/sound/soc/intel/avs/core.c
> @@ -473,8 +473,13 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   	}
>   
>   	snd_hdac_bus_parse_capabilities(bus);
> -	if (bus->mlcap)
> -		snd_hdac_ext_bus_get_ml_capabilities(bus);
> +	if (bus->mlcap) {
> +		ret = snd_hdac_ext_bus_get_ml_capabilities(bus);

After giving this a second thought, I believe 
snd_hdac_ext_bus_get_ml_capabilities() is the offender here - the 
function should have freed whatever its already allocated before 
returning an error, not count on the caller to free the resources 
instead. In other words, the fix should update the callee too.

However, one may say that it's a separate issue. I'm fine with existing 
patch landing as-is. Can prepare separate a change that covers problem 
mentioned by me above. The cons is: additional 1-2 LOC traffic for the 
avs-driver code.

I leave the decision to Mark, I'm OK with both approaches.

> +		if (ret) {
> +			dev_err(dev, "failed to get multilink capabilities: %d\n", ret);
> +			goto err_ml_capabilities;
> +		}
> +	}
>   
>   	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)))
>   		dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> @@ -483,7 +488,7 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   	ret = avs_hdac_bus_init_streams(bus);
>   	if (ret < 0) {
>   		dev_err(dev, "failed to init streams: %d\n", ret);
> -		goto err_init_streams;
> +		goto err_ml_capabilities;
>   	}
>   
>   	ret = avs_hdac_acquire_irq(adev);
> @@ -515,7 +520,8 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
>   err_acquire_irq:
>   	snd_hdac_bus_free_stream_pages(bus);
>   	snd_hdac_ext_stream_free_all(bus);
> -err_init_streams:
> +err_ml_capabilities:
> +	snd_hdac_ext_link_free_all(bus);
>   	iounmap(adev->dsp_ba);
>   err_remap_bar4:
>   	iounmap(bus->remap_addr);