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

[Cezary Rojewski]

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