[v2] ALSA: hda: Abort capability probe on invalid capability
diff mbox

Message ID 1508370325-3118-1-git-send-email-rakesh.a.ughreja@intel.com
State New
Headers show

Commit Message

Ughreja, Rakesh A Oct. 18, 2017, 11:45 p.m. UTC
From: Rakesh Ughreja <rakesh.a.ughreja@intel.com>

Due to bugs in BIOS it's possible that the HDA capability link
list is not constructed properly. This may lead to driver going
into unknown state. So whenever driver discovers unknown HDA
capability, log it as error and stop traversing the link list
further.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/hda/hdac_controller.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Vinod Koul Oct. 18, 2017, 4:26 p.m. UTC | #1
On Thu, Oct 19, 2017 at 05:15:25AM +0530, Ughreja, Rakesh A wrote:
> From: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> 
> Due to bugs in BIOS it's possible that the HDA capability link

It maybe due to BIOS bug or some other issue, lets not quantify that please.

I would say "On reading wrong capablity pointer values we may crash...

> list is not constructed properly. This may lead to driver going
> into unknown state. 

I dont think we maintain driver states which are going wrong here

> So whenever driver discovers unknown HDA
> capability, log it as error and stop traversing the link list
> further.

that sound good

> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>  sound/hda/hdac_controller.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
> index 978dc18..8f7d0d9 100644
> --- a/sound/hda/hdac_controller.c
> +++ b/sound/hda/hdac_controller.c
> @@ -314,7 +314,8 @@ int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
>  			break;
>  
>  		default:
> -			dev_dbg(bus->dev, "Unknown capability %d\n", cur_cap);
> +			dev_err(bus->dev, "Unknown capability %d\n", cur_cap);
> +			cur_cap = 0;
>  			break;
>  		}
>  
> -- 
> 2.7.4
>
Ughreja, Rakesh A Oct. 18, 2017, 4:52 p.m. UTC | #2
>-----Original Message-----
>From: Koul, Vinod
>Sent: Wednesday, October 18, 2017 9:57 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>
>Cc: alsa-devel@alsa-project.org; tiwai@suse.de
>Subject: Re: [PATCH v2] ALSA: hda: Abort capability probe on invalid capability
>
>On Thu, Oct 19, 2017 at 05:15:25AM +0530, Ughreja, Rakesh A wrote:
>> From: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>
>> Due to bugs in BIOS it's possible that the HDA capability link
>
>It maybe due to BIOS bug or some other issue, lets not quantify that please.
>
>I would say "On reading wrong capablity pointer values we may crash...
>
>> list is not constructed properly. This may lead to driver going
>> into unknown state.
>
>I dont think we maintain driver states which are going wrong here
>
>> So whenever driver discovers unknown HDA
>> capability, log it as error and stop traversing the link list
>> further.
>
>that sound good

Hi Vinod, Takashi,

Are you both ok with following commit message ?

On reading wrong capability pointer values driver may crash, so 
whenever driver discovers unknown HDA capability, log it as error 
and stop traversing the link list further.

Regards,
Rakesh
Takashi Iwai Oct. 19, 2017, 7:15 a.m. UTC | #3
On Wed, 18 Oct 2017 18:52:54 +0200,
Ughreja, Rakesh A wrote:
> 
> 
> 
> >-----Original Message-----
> >From: Koul, Vinod
> >Sent: Wednesday, October 18, 2017 9:57 PM
> >To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>
> >Cc: alsa-devel@alsa-project.org; tiwai@suse.de
> >Subject: Re: [PATCH v2] ALSA: hda: Abort capability probe on invalid capability
> >
> >On Thu, Oct 19, 2017 at 05:15:25AM +0530, Ughreja, Rakesh A wrote:
> >> From: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> >>
> >> Due to bugs in BIOS it's possible that the HDA capability link
> >
> >It maybe due to BIOS bug or some other issue, lets not quantify that please.
> >
> >I would say "On reading wrong capablity pointer values we may crash...
> >
> >> list is not constructed properly. This may lead to driver going
> >> into unknown state.
> >
> >I dont think we maintain driver states which are going wrong here
> >
> >> So whenever driver discovers unknown HDA
> >> capability, log it as error and stop traversing the link list
> >> further.
> >
> >that sound good
> 
> Hi Vinod, Takashi,
> 
> Are you both ok with following commit message ?
> 
> On reading wrong capability pointer values driver may crash, so 
> whenever driver discovers unknown HDA capability, log it as error 
> and stop traversing the link list further.

Looks good to me.


thanks,

Takashi

Patch
diff mbox

diff --git a/sound/hda/hdac_controller.c b/sound/hda/hdac_controller.c
index 978dc18..8f7d0d9 100644
--- a/sound/hda/hdac_controller.c
+++ b/sound/hda/hdac_controller.c
@@ -314,7 +314,8 @@  int snd_hdac_bus_parse_capabilities(struct hdac_bus *bus)
 			break;
 
 		default:
-			dev_dbg(bus->dev, "Unknown capability %d\n", cur_cap);
+			dev_err(bus->dev, "Unknown capability %d\n", cur_cap);
+			cur_cap = 0;
 			break;
 		}