Re: [External] Re: [PATCH] ALSA: hda: ignore invalid NHLT table

From: Mark Pearson <markpearson@lenovo.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, Takashi Iwai <tiwai@suse.de>,
	"Rojewski, Cezary" <cezary.rojewski@intel.com>,
	Mark Brown <broonie@kernel.org>,
	Philipp Leskovitz <philipp.leskovitz@secunet.com>
Subject: Re: [External] Re: [PATCH] ALSA: hda: ignore invalid NHLT table
Date: Tue, 2 Mar 2021 10:57:43 -0500	[thread overview]
Message-ID: <55a6f9e6-17ab-6049-d2e2-5dee1941b539@lenovo.com> (raw)
In-Reply-To: <420b9b48-0a80-634b-9a98-973c6bdb544c@linux.intel.com>

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++) {
>>