Re: [PATCH v2] ASoC: intel: avs: Fix potential memory leak in avs_pci_probe()

[Cezary Rojewski <cezary.rojewski@intel.com>]

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