[alsa-devel] [PATCH] ALSA: hda: Abort capability probe at invalid register read

[Vinod Koul]

On Tue, Oct 17, 2017 at 07:15:08PM +0200, Takashi Iwai wrote:
> On Tue, 17 Oct 2017 19:15:08 +0200,
> Vinod Koul wrote:
> > 
> > On Tue, Oct 17, 2017 at 04:47:11PM +0200, Takashi Iwai wrote:
> > > The loop in snd_hdac_bus_parse_capabilities() may go to nirvana when
> > > it hits an invalid register value read:
> > > 
> > >  BUG: unable to handle kernel paging request at ffffad5dc41f3fff
> > >  IP: pci_azx_readl+0x5/0x10 [snd_hda_intel]
> > >  Call Trace:
> > >   snd_hdac_bus_parse_capabilities+0x3c/0x1f0 [snd_hda_core]
> > >   azx_probe_continue+0x7d5/0x940 [snd_hda_intel]
> > >   .....
> > > 
> > > This happened on a new Intel machine, and we need to check the value
> > > and abort the loop accordingly.
> > 
> > okay and what machine is the problem here. I have had a similar bug report
> > from Gfx CI guys on  CFL machine. Turns out the BIOS was buggy and we fixed
> > that up by upgrading the BIOS.
> 
> Yes, it's a CFL-H.  Possibly a buggy BIOS, but the driver still
> shouldn't crash.

Okay so can you ask them to update BIOS and check.

> > Yes it is a good idea to keep this guard but -1 would mean that HW read is
> > failing which points to some other issue here
> 
> Right.

In this case should we send this to stable? I have not seen this crashing
till now except bad BIOS issue

> 
> 
> Takashi
> 
> > > [Note: the fixes tag below indicates only the commit where this patch
> > >  can be applied; the original problem was introduced even before that
> > >  commit]
> > > 
> > > Fixes: 6720b38420a0 ("ALSA: hda - move bus_parse_capabilities to core")
> > > Cc: <stable at vger.kernel.org>
> > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > ---
> > >  sound/hda/hdac_controller.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> > > index 978dc1801b3a..f6d2985b2520 100644
> > > --- a/sound/hda/hdac_controller.c
> > > +++ b/sound/hda/hdac_controller.c
> > > @@ -284,6 +284,11 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
> > >  		dev_dbg(bus->dev, "HDA capability ID: 0x%x\n",
> > >  			(cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF);
> > >  
> > > +		if (cur_cap == -1) {
> > > +			dev_dbg(bus->dev, "Invalid capability reg read\n");
> > > +			break;
> > > +		}
> > > +
> > >  		switch ((cur_cap & AZX_CAP_HDR_ID_MASK) >> AZX_CAP_HDR_ID_OFF) {
> > >  		case AZX_ML_CAP_ID:
> > >  			dev_dbg(bus->dev, "Found ML capability\n");
> > > -- 
> > > 2.14.2
> > > 
> > 
> > -- 
> > ~Vinod
> > 

-- 
~Vinod