[PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Armin Wolf]

It turns out that the ECDT table inside the ThinkBook 14 G7 IML
contains a valid EC description but an invalid ID string
("_SB.PC00.LPCB.EC0"). Ignoring this ECDT based on the invalid
ID string prevents the kernel from detecting the built-in touchpad,
so relax the sanity check of the ID string and only reject ECDTs
with empty ID strings.

Compile-tested only.

Reported-by: Ilya K <me@0upti.me>
Fixes: 7a0d59f6a913 ("ACPI: EC: Ignore ECDT tables with an invalid ID string")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/ec.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 75c7db8b156a..7855bbf752b1 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -2033,7 +2033,7 @@ void __init acpi_ec_ecdt_probe(void)
 		goto out;
 	}
 
-	if (!strstarts(ecdt_ptr->id, "\\")) {
+	if (!strlen(ecdt_ptr->id)) {
 		/*
 		 * The ECDT table on some MSI notebooks contains invalid data, together
 		 * with an empty ID string ("").
@@ -2042,9 +2042,13 @@ void __init acpi_ec_ecdt_probe(void)
 		 * a "fully qualified reference to the (...) embedded controller device",
 		 * so this string always has to start with a backslash.
 		 *
-		 * By verifying this we can avoid such faulty ECDT tables in a safe way.
+		 * However some ThinkBook machines have a ECDT table with a valid EC
+		 * description but an invalid ID string ("_SB.PC00.LPCB.EC0").
+		 *
+		 * Because of this we only check if the ID string is empty in order to
+		 * avoid the obvious cases.
 		 */
-		pr_err(FW_BUG "Ignoring ECDT due to invalid ID string \"%s\"\n", ecdt_ptr->id);
+		pr_err(FW_BUG "Ignoring ECDT due to empty ID string\n");
 		goto out;
 	}
 
-- 
2.39.5

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Ilya K]

On 2025-07-29 09:20, Armin Wolf wrote:
> It turns out that the ECDT table inside the ThinkBook 14 G7 IML
> contains a valid EC description but an invalid ID string
> ("_SB.PC00.LPCB.EC0"). Ignoring this ECDT based on the invalid
> ID string prevents the kernel from detecting the built-in touchpad,
> so relax the sanity check of the ID string and only reject ECDTs
> with empty ID strings.
> 
> Compile-tested only.
> 
> Reported-by: Ilya K <me@0upti.me>
> Fixes: 7a0d59f6a913 ("ACPI: EC: Ignore ECDT tables with an invalid ID string")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/acpi/ec.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 

Thanks, this works!

Tested-by: Ilya K <me@0upti.me>

> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index 75c7db8b156a..7855bbf752b1 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -2033,7 +2033,7 @@ void __init acpi_ec_ecdt_probe(void)
>  		goto out;
>  	}
>  
> -	if (!strstarts(ecdt_ptr->id, "\\")) {
> +	if (!strlen(ecdt_ptr->id)) {
>  		/*
>  		 * The ECDT table on some MSI notebooks contains invalid data, together
>  		 * with an empty ID string ("").
> @@ -2042,9 +2042,13 @@ void __init acpi_ec_ecdt_probe(void)
>  		 * a "fully qualified reference to the (...) embedded controller device",
>  		 * so this string always has to start with a backslash.
>  		 *
> -		 * By verifying this we can avoid such faulty ECDT tables in a safe way.
> +		 * However some ThinkBook machines have a ECDT table with a valid EC
> +		 * description but an invalid ID string ("_SB.PC00.LPCB.EC0").
> +		 *
> +		 * Because of this we only check if the ID string is empty in order to
> +		 * avoid the obvious cases.
>  		 */
> -		pr_err(FW_BUG "Ignoring ECDT due to invalid ID string \"%s\"\n", ecdt_ptr->id);
> +		pr_err(FW_BUG "Ignoring ECDT due to empty ID string\n");
>  		goto out;
>  	}
>  

Would it maybe make sense to also log a warning for the old case? Maybe a vendor will notice it and fix the firmware...
(yeah yeah fat chance)

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Armin Wolf]

Am 29.07.25 um 09:00 schrieb Ilya K:

> On 2025-07-29 09:20, Armin Wolf wrote:
>> It turns out that the ECDT table inside the ThinkBook 14 G7 IML
>> contains a valid EC description but an invalid ID string
>> ("_SB.PC00.LPCB.EC0"). Ignoring this ECDT based on the invalid
>> ID string prevents the kernel from detecting the built-in touchpad,
>> so relax the sanity check of the ID string and only reject ECDTs
>> with empty ID strings.
>>
>> Compile-tested only.
>>
>> Reported-by: Ilya K <me@0upti.me>
>> Fixes: 7a0d59f6a913 ("ACPI: EC: Ignore ECDT tables with an invalid ID string")
>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> ---
>>   drivers/acpi/ec.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
> Thanks, this works!
>
> Tested-by: Ilya K <me@0upti.me>
>
>> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
>> index 75c7db8b156a..7855bbf752b1 100644
>> --- a/drivers/acpi/ec.c
>> +++ b/drivers/acpi/ec.c
>> @@ -2033,7 +2033,7 @@ void __init acpi_ec_ecdt_probe(void)
>>   		goto out;
>>   	}
>>   
>> -	if (!strstarts(ecdt_ptr->id, "\\")) {
>> +	if (!strlen(ecdt_ptr->id)) {
>>   		/*
>>   		 * The ECDT table on some MSI notebooks contains invalid data, together
>>   		 * with an empty ID string ("").
>> @@ -2042,9 +2042,13 @@ void __init acpi_ec_ecdt_probe(void)
>>   		 * a "fully qualified reference to the (...) embedded controller device",
>>   		 * so this string always has to start with a backslash.
>>   		 *
>> -		 * By verifying this we can avoid such faulty ECDT tables in a safe way.
>> +		 * However some ThinkBook machines have a ECDT table with a valid EC
>> +		 * description but an invalid ID string ("_SB.PC00.LPCB.EC0").
>> +		 *
>> +		 * Because of this we only check if the ID string is empty in order to
>> +		 * avoid the obvious cases.
>>   		 */
>> -		pr_err(FW_BUG "Ignoring ECDT due to invalid ID string \"%s\"\n", ecdt_ptr->id);
>> +		pr_err(FW_BUG "Ignoring ECDT due to empty ID string\n");
>>   		goto out;
>>   	}
>>   
> Would it maybe make sense to also log a warning for the old case? Maybe a vendor will notice it and fix the firmware...
> (yeah yeah fat chance)

The Linux kernel is not a verification kit, so i am against keeping the old check. Instead i suggest that we ensure that
the FWTS project (https://github.com/fwts/fwts) detects such invalid ECDT tables. Can you share the full output of
acpidump so that i can run the fwts tool on it?

Thanks,
Armin Wolf

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Ilya K]

> The Linux kernel is not a verification kit, so i am against keeping the old check. Instead i suggest that we ensure that
> the FWTS project (https://github.com/fwts/fwts) detects such invalid ECDT tables. Can you share the full output of
> acpidump so that i can run the fwts tool on it?
> 
Uploaded here: https://github.com/K900/21mr-acpi-dumps

Thanks!

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Armin Wolf]

Am 30.07.25 um 18:59 schrieb Ilya K:

>> The Linux kernel is not a verification kit, so i am against keeping the old check. Instead i suggest that we ensure that
>> the FWTS project (https://github.com/fwts/fwts) detects such invalid ECDT tables. Can you share the full output of
>> acpidump so that i can run the fwts tool on it?
>>
> Uploaded here: https://github.com/K900/21mr-acpi-dumps
>
> Thanks!

FWTS already warns that the ID string is wrong, so all good on this site.

Thanks,
Armin Wolf

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Ilya K]

> 
> FWTS already warns that the ID string is wrong, so all good on this site.
> 

Is there anything else I can do to help get this merged? It's a pretty major regression and it would be good to have it in the next stable.

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Rafael J. Wysocki]

On Tue, Jul 29, 2025 at 9:01 AM Ilya K <me@0upti.me> wrote:
>
> On 2025-07-29 09:20, Armin Wolf wrote:
> > It turns out that the ECDT table inside the ThinkBook 14 G7 IML
> > contains a valid EC description but an invalid ID string
> > ("_SB.PC00.LPCB.EC0"). Ignoring this ECDT based on the invalid
> > ID string prevents the kernel from detecting the built-in touchpad,
> > so relax the sanity check of the ID string and only reject ECDTs
> > with empty ID strings.
> >
> > Compile-tested only.
> >
> > Reported-by: Ilya K <me@0upti.me>
> > Fixes: 7a0d59f6a913 ("ACPI: EC: Ignore ECDT tables with an invalid ID string")
> > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> > ---
> >  drivers/acpi/ec.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
>
> Thanks, this works!
>
> Tested-by: Ilya K <me@0upti.me>

Applied as 6.17-rc material and sorry for the delay (I was offline).

Thanks!

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Ilya K]

On 2025-08-12 16:32, Rafael J. Wysocki wrote:
> 
> Applied as 6.17-rc material and sorry for the delay (I was offline).
> 
> Thanks!

Thanks!

Tagging stable@ so we're hopefully in time for 6.16.1.

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Greg KH]

On Tue, Aug 12, 2025 at 06:51:10PM +0300, Ilya K wrote:
> On 2025-08-12 16:32, Rafael J. Wysocki wrote:
> > 
> > Applied as 6.17-rc material and sorry for the delay (I was offline).
> > 
> > Thanks!
> 
> Thanks!
> 
> Tagging stable@ so we're hopefully in time for 6.16.1.

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Armin Wolf]

Am 12.08.25 um 18:40 schrieb Greg KH:

> On Tue, Aug 12, 2025 at 06:51:10PM +0300, Ilya K wrote:
>> On 2025-08-12 16:32, Rafael J. Wysocki wrote:
>>> Applied as 6.17-rc material and sorry for the delay (I was offline).
>>>
>>> Thanks!
>> Thanks!
>>
>> Tagging stable@ so we're hopefully in time for 6.16.1.
> <formletter>
>
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
>
> </formletter>

Agree.

AFAIK the Fixes: tag should be enough to ensure that this patch gets included
in the affected stable kernels.

Thanks,
Armin Wolf
  

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Greg KH]

On Tue, Aug 12, 2025 at 06:54:39PM +0200, Armin Wolf wrote:
> Am 12.08.25 um 18:40 schrieb Greg KH:
> 
> > On Tue, Aug 12, 2025 at 06:51:10PM +0300, Ilya K wrote:
> > > On 2025-08-12 16:32, Rafael J. Wysocki wrote:
> > > > Applied as 6.17-rc material and sorry for the delay (I was offline).
> > > > 
> > > > Thanks!
> > > Thanks!
> > > 
> > > Tagging stable@ so we're hopefully in time for 6.16.1.
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Agree.
> 
> AFAIK the Fixes: tag should be enough to ensure that this patch gets included
> in the affected stable kernels.

Not at all!

Please read the above link for the full details on how to do this (hint,
Fixes: will not do it.)

thanks,

greg k-h

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Ilya K]

On 2025-08-12 20:10, Greg KH wrote:
> 
> Please read the above link for the full details on how to do this (hint,
> Fixes: will not do it.)
> 

I might be missing something, but doesn't that just tell you to CC stable@?
Or do you have to specifically have the CC on the initial patch submission, not anywhere in the thread?

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Rafael J. Wysocki]

On Tue, Aug 12, 2025 at 7:57 PM Ilya K <me@0upti.me> wrote:
>
> On 2025-08-12 20:10, Greg KH wrote:
> >
> > Please read the above link for the full details on how to do this (hint,
> > Fixes: will not do it.)
> >
>
> I might be missing something, but doesn't that just tell you to CC stable@?
> Or do you have to specifically have the CC on the initial patch submission, not anywhere in the thread?

A Cc: stable in a patch (or elsewhere in the thread following it) is
just a hint for the maintainer anyway.

But no worries, I've added Cc: stable to the commit.

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Greg KH]

On Tue, Aug 12, 2025 at 08:56:55PM +0300, Ilya K wrote:
> On 2025-08-12 20:10, Greg KH wrote:
> > 
> > Please read the above link for the full details on how to do this (hint,
> > Fixes: will not do it.)
> > 
> 
> I might be missing something, but doesn't that just tell you to CC stable@?

It must be in the patch itself, in the signed-off-by area.

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Rafael J. Wysocki]

On Tue, Aug 12, 2025 at 9:33 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Aug 12, 2025 at 08:56:55PM +0300, Ilya K wrote:
> > On 2025-08-12 20:10, Greg KH wrote:
> > >
> > > Please read the above link for the full details on how to do this (hint,
> > > Fixes: will not do it.)
> > >
> >
> > I might be missing something, but doesn't that just tell you to CC stable@?
>
> It must be in the patch itself, in the signed-off-by area.

Well, to be precise, it must be present in the mainline commit of that
patch and whether or not it will be there is entirely up to the
committer.

So adding a Cc: stable tag to a patch is a hint, which usually is
appreciated as long as it is accurate, but the committer may still
decide to not include it in the commit.

Re: [PATCH] ACPI: EC: Relax sanity check of the ECDT ID string

[Armin Wolf]

Am 12.08.25 um 19:10 schrieb Greg KH:

> On Tue, Aug 12, 2025 at 06:54:39PM +0200, Armin Wolf wrote:
>> Am 12.08.25 um 18:40 schrieb Greg KH:
>>
>>> On Tue, Aug 12, 2025 at 06:51:10PM +0300, Ilya K wrote:
>>>> On 2025-08-12 16:32, Rafael J. Wysocki wrote:
>>>>> Applied as 6.17-rc material and sorry for the delay (I was offline).
>>>>>
>>>>> Thanks!
>>>> Thanks!
>>>>
>>>> Tagging stable@ so we're hopefully in time for 6.16.1.
>>> <formletter>
>>>
>>> This is not the correct way to submit patches for inclusion in the
>>> stable kernel tree.  Please read:
>>>       https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>> for how to do this properly.
>>>
>>> </formletter>
>> Agree.
>>
>> AFAIK the Fixes: tag should be enough to ensure that this patch gets included
>> in the affected stable kernels.
> Not at all!
>
> Please read the above link for the full details on how to do this (hint,
> Fixes: will not do it.)
>
> thanks,
>
> greg k-h

Oops, seems that i missed something rather significant about the stable kernels.
I will give keep that in mind when sending future patches.

Thanks,
Armin Wolf