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@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@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@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel