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: <20910af9-271e-431f-a896-b5216fed1c1c@intel.com>
Date: Fri, 21 Nov 2025 09:55:49 +0100
From: Cezary Rojewski <cezary.rojewski@...el.com>
To: Mark Brown <broonie@...nel.org>
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>,
	<perex@...ex.cz>, <tiwai@...e.com>, <linux-sound@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Abdun Nihaal <nihaal@....iitm.ac.in>
Subject: Re: [PATCH v2] ASoC: intel: avs: Fix potential memory leak in
 avs_pci_probe()

On 2025-11-13 2:11 PM, Cezary Rojewski wrote:
> 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().

...
>> 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.

Friendly reminder. Which option do you prefer, Mark?

> 
>> +        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));