[alsa-devel] [PATCH] ASoC: Intel: Skylake: Acquire irq after RIRB allocation

[Takashi Iwai]

On Tue, 14 Aug 2018 16:50:51 +0200,
Pierre-Louis Bossart wrote:
> 
> On 8/13/18 6:15 PM, Yong Zhi wrote:
> > Cold reboot stress test found that the hda irq could access rirb ring
> > buffer before its memory gets allocated which resulting in null
> > pointer dereference inside snd_hdac_bus_update_rirb().
> >
> > Fix it by moving the skl_acquire_irq after ring buffer allocation.
> > While here, also change err return from -EBUSY to actual error code.
> 
> I am not that familiar with PCI gory details but that patch was
> reviewed internally with no objections raised; there was also an
> agreement that the SOF driver would follow the same sequence, so
> 
> Acked-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>

It's a standard idiom for a driver allocating a shared irq line.
Because an irq may be issued by another device on the same line, the
registered irq handler may be kicked off before the registers or
whatever else is ready for use, eventually leading to some Oops.

The destructor is other way round; first free the irq handler, then
release the rest resources.


Takashi

> 
> >
> > Signed-off-by: Yong Zhi <yong.zhi at intel.com>
> > ---
> >   sound/soc/intel/skylake/skl.c | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> > index dce649485649..cf09721ca13e 100644
> > --- a/sound/soc/intel/skylake/skl.c
> > +++ b/sound/soc/intel/skylake/skl.c
> > @@ -838,11 +838,7 @@ static int skl_first_init(struct hdac_bus *bus)
> >     	snd_hdac_bus_parse_capabilities(bus);
> >   -	if (skl_acquire_irq(bus, 0) < 0)
> > -		return -EBUSY;
> > -
> >   	pci_set_master(pci);
> > -	synchronize_irq(bus->irq);
> >     	gcap = snd_hdac_chip_readw(bus, GCAP);
> >   	dev_dbg(bus->dev, "chipset global capabilities = 0x%x\n", gcap);
> > @@ -875,6 +871,12 @@ static int skl_first_init(struct hdac_bus *bus)
> >   	if (err < 0)
> >   		return err;
> >   +	err = skl_acquire_irq(bus, 0);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	synchronize_irq(bus->irq);
> > +
> >   	/* initialize chip */
> >   	skl_init_pci(skl);
> >   
> >
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>