alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ALSA: hda: ignore invalid NHLT table
@ 2021-03-02 14:10 Mark Pearson
  2021-03-02 14:46 ` Pierre-Louis Bossart
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Pearson @ 2021-03-02 14:10 UTC (permalink / raw)
  To: markpearson; +Cc: alsa-devel, Philipp Leskovitz

On some Lenovo systems if the microphone is disabled in the BIOS
only the NHLT table header is created, with no data. This means
the endpoints field is not correctly set to zero - leading to an
unintialised variable and hence invalid descriptors are parsed
leading to page faults.

The Lenovo firmware team is addressing this, but adding a check
preventing invalid tables being parsed is worthwhile.

Tested on a Lenovo T14.

Tested-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
Reported-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
 sound/hda/intel-nhlt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index 059aaf04f..0889f2cc5 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -37,6 +37,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 	if (!nhlt)
 		return 0;
 
+	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
+		dev_warn(dev, "Invalid DMIC description table\n");
+		return 0;
+	}
+
 	epnt = (struct nhlt_endpoint *)nhlt->desc;
 
 	for (j = 0; j < nhlt->endpoint_count; j++) {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ALSA: hda: ignore invalid NHLT table
  2021-03-02 14:10 [PATCH] ALSA: hda: ignore invalid NHLT table Mark Pearson
@ 2021-03-02 14:46 ` Pierre-Louis Bossart
  2021-03-02 15:57   ` [External] " Mark Pearson
  0 siblings, 1 reply; 6+ messages in thread
From: Pierre-Louis Bossart @ 2021-03-02 14:46 UTC (permalink / raw)
  To: Mark Pearson
  Cc: alsa-devel, Takashi Iwai, Rojewski, Cezary, Mark Brown,
	Philipp Leskovitz

Adding Mark, Takashi, Jaroslav and Cezary in Cc:

On 3/2/21 8:10 AM, Mark Pearson wrote:
> On some Lenovo systems if the microphone is disabled in the BIOS
> only the NHLT table header is created, with no data. This means
> the endpoints field is not correctly set to zero - leading to an
> unintialised variable and hence invalid descriptors are parsed
> leading to page faults.
> 
> The Lenovo firmware team is addressing this, but adding a check
> preventing invalid tables being parsed is worthwhile.
> 
> Tested on a Lenovo T14.
> 
> Tested-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
> Reported-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
> Signed-off-by: Mark Pearson <markpearson@lenovo.com>

The change looks good to me

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Out of curiosity, does this impact Kabylake or CometLake+ systems?

Thanks!

> ---
>   sound/hda/intel-nhlt.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
> index 059aaf04f..0889f2cc5 100644
> --- a/sound/hda/intel-nhlt.c
> +++ b/sound/hda/intel-nhlt.c
> @@ -37,6 +37,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
>   	if (!nhlt)
>   		return 0;
>   
> +	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
> +		dev_warn(dev, "Invalid DMIC description table\n");
> +		return 0;
> +	}
> +
>   	epnt = (struct nhlt_endpoint *)nhlt->desc;
>   
>   	for (j = 0; j < nhlt->endpoint_count; j++) {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] Re: [PATCH] ALSA: hda: ignore invalid NHLT table
  2021-03-02 14:46 ` Pierre-Louis Bossart
@ 2021-03-02 15:57   ` Mark Pearson
  2021-03-04  8:27     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Pearson @ 2021-03-02 15:57 UTC (permalink / raw)
  To: Pierre-Louis Bossart
  Cc: alsa-devel, Takashi Iwai, Rojewski, Cezary, Mark Brown,
	Philipp Leskovitz

Hi Pierre-Louis,

On 02/03/2021 09:46, Pierre-Louis Bossart wrote:
> Adding Mark, Takashi, Jaroslav and Cezary in Cc:

Thanks - I wasn't sure who needed to be on this :)
> 
> On 3/2/21 8:10 AM, Mark Pearson wrote:
>> On some Lenovo systems if the microphone is disabled in the BIOS
>> only the NHLT table header is created, with no data. This means
>> the endpoints field is not correctly set to zero - leading to an
>> unintialised variable and hence invalid descriptors are parsed
>> leading to page faults.
>>
>> The Lenovo firmware team is addressing this, but adding a check
>> preventing invalid tables being parsed is worthwhile.
>>
>> Tested on a Lenovo T14.
>>
>> Tested-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
>> Reported-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
>> Signed-off-by: Mark Pearson <markpearson@lenovo.com>
> 
> The change looks good to me
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> Out of curiosity, does this impact Kabylake or CometLake+ systems?

We've seen it on some of our CometLake systems so far. T14 & T15 are
confirmed, but it will take a while to cycle through all the platforms.
We're unlikely to catch any non-Linux certified systems that folk still
put Linux on - I'd like to have this available for them too.

> 
> Thanks!
> 
>> ---
>>   sound/hda/intel-nhlt.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
>> index 059aaf04f..0889f2cc5 100644
>> --- a/sound/hda/intel-nhlt.c
>> +++ b/sound/hda/intel-nhlt.c
>> @@ -37,6 +37,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev,
>> struct nhlt_acpi_table *nhlt)
>>       if (!nhlt)
>>           return 0;
>>   +    if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
>> +        dev_warn(dev, "Invalid DMIC description table\n");
>> +        return 0;
>> +    }
>> +
>>       epnt = (struct nhlt_endpoint *)nhlt->desc;
>>         for (j = 0; j < nhlt->endpoint_count; j++) {
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] Re: [PATCH] ALSA: hda: ignore invalid NHLT table
  2021-03-02 15:57   ` [External] " Mark Pearson
@ 2021-03-04  8:27     ` Takashi Iwai
  2021-03-04 14:30       ` Mark Pearson
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2021-03-04  8:27 UTC (permalink / raw)
  To: Mark Pearson
  Cc: Rojewski, Cezary, alsa-devel, Mark Brown, Philipp Leskovitz,
	Pierre-Louis Bossart

On Tue, 02 Mar 2021 16:57:43 +0100,
Mark Pearson wrote:
> 
> Hi Pierre-Louis,
> 
> On 02/03/2021 09:46, Pierre-Louis Bossart wrote:
> > Adding Mark, Takashi, Jaroslav and Cezary in Cc:
> 
> Thanks - I wasn't sure who needed to be on this :)

Thanks, applied now.

Unfortunately the patch conflicted slightly with the recent change by
Pierre, so I corrected in my side.  Please check it later.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [External] Re: [PATCH] ALSA: hda: ignore invalid NHLT table
  2021-03-04  8:27     ` Takashi Iwai
@ 2021-03-04 14:30       ` Mark Pearson
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Pearson @ 2021-03-04 14:30 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Rojewski, Cezary, alsa-devel, Mark Brown, Philipp Leskovitz,
	Pierre-Louis Bossart


On 04/03/2021 03:27, Takashi Iwai wrote:
> On Tue, 02 Mar 2021 16:57:43 +0100,
> Mark Pearson wrote:
>>
>> Hi Pierre-Louis,
>>
>> On 02/03/2021 09:46, Pierre-Louis Bossart wrote:
>>> Adding Mark, Takashi, Jaroslav and Cezary in Cc:
>>
>> Thanks - I wasn't sure who needed to be on this :)
> 
> Thanks, applied now.
> 
> Unfortunately the patch conflicted slightly with the recent change by
> Pierre, so I corrected in my side.  Please check it later.
> 
Will do.

Thanks!
Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ALSA: hda: ignore invalid NHLT table
@ 2021-03-02 14:41 Mark Pearson
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Pearson @ 2021-03-02 14:41 UTC (permalink / raw)
  To: markpearson; +Cc: alsa-devel, Philipp Leskovitz

On some Lenovo systems if the microphone is disabled in the BIOS
only the NHLT table header is created, with no data. This means
the endpoints field is not correctly set to zero - leading to an
unintialised variable and hence invalid descriptors are parsed
leading to page faults.

The Lenovo firmware team is addressing this, but adding a check
preventing invalid tables being parsed is worthwhile.

Tested on a Lenovo T14.

Tested-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
Reported-by: Philipp Leskovitz <philipp.leskovitz@secunet.com>
Signed-off-by: Mark Pearson <markpearson@lenovo.com>
---
 sound/hda/intel-nhlt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/sound/hda/intel-nhlt.c b/sound/hda/intel-nhlt.c
index 059aaf04f..0889f2cc5 100644
--- a/sound/hda/intel-nhlt.c
+++ b/sound/hda/intel-nhlt.c
@@ -37,6 +37,11 @@ int intel_nhlt_get_dmic_geo(struct device *dev, struct nhlt_acpi_table *nhlt)
 	if (!nhlt)
 		return 0;
 
+	if (nhlt->header.length <= sizeof(struct acpi_table_header)) {
+		dev_warn(dev, "Invalid DMIC description table\n");
+		return 0;
+	}
+
 	epnt = (struct nhlt_endpoint *)nhlt->desc;
 
 	for (j = 0; j < nhlt->endpoint_count; j++) {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-04 14:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 14:10 [PATCH] ALSA: hda: ignore invalid NHLT table Mark Pearson
2021-03-02 14:46 ` Pierre-Louis Bossart
2021-03-02 15:57   ` [External] " Mark Pearson
2021-03-04  8:27     ` Takashi Iwai
2021-03-04 14:30       ` Mark Pearson
2021-03-02 14:41 Mark Pearson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).