linux-kernel - Re: [PATCH v2] platform/x86: toshiba_acpi: Fix quickstart quirk handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d2b98aab-2f71-4810-b706-78dffb0649b9@gmx.de>
Date: Tue, 2 Jul 2024 13:13:06 +0200
From: Armin Wolf <W_Armin@....de>
To: Hans de Goede <hdegoede@...hat.com>, coproscefalo@...il.com
Cc: lkml@...pal.se, kmal@...k.li, ilpo.jarvinen@...ux.intel.com,
 platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] platform/x86: toshiba_acpi: Fix quickstart quirk
 handling

Am 02.07.24 um 12:52 schrieb Hans de Goede:

> Hi Armin,
>
> On 7/1/24 9:45 PM, Armin Wolf wrote:
>> The global hci_hotkey_quickstart quirk flag is tested in
>> toshiba_acpi_enable_hotkeys() before the quirk flag is properly
>> initialized based on SMBIOS data. This causes the quirk to be
>> applied to all models, some of which behave erratically as a
>> result.
>>
>> Fix this by initializing the global quirk flags during module
>> initialization before registering the ACPI driver. This also
>> allows us to mark toshiba_dmi_quirks[] as __initconst.
>>
>> Fixes: 23f1d8b47d12 ("platform/x86: toshiba_acpi: Add quirk for buttons on Z830")
>> Reported-by: kemal <kmal@...k.li>
>> Tested-by: kemal <kmal@...k.li>
>> Signed-off-by: Armin Wolf <W_Armin@....de>
>> ---
>> Changes since v1:
>>   - add Tested-by tag
> Thank you for fixing this, the patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
>
> I was hoping my pdx86 fixes pull-request last Saturday would
> be the last one for this cycle, but I'll prep another one
> with this patch sometime this week:
>
> Thank you for your patch/series, I've applied this patch
> (series) to my review-hans branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>
> I have added a:
>
> Closes: https://lore.kernel.org/platform-driver-x86/R4CYFS.TWB8QUU2SHWI1@cock.li/
>
> tag whole applying this.
>
> I will include this patch in my next fixes pull-req to Linus
> for the current kernel development cycle.
>
> Regards,
>
> Hans
>
>
>
> Regards,
>
> Hans

Thanks,

i will contact the stable team and ask them to revert the faulty commit from the stable kernels,
since the commit only makes sense together with the quickstart driver.

Thanks,
Armin Wolf

>
>
>> ---
>>   drivers/platform/x86/toshiba_acpi.c | 31 +++++++++++++++++------------
>>   1 file changed, 18 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/platform/x86/toshiba_acpi.c b/drivers/platform/x86/toshiba_acpi.c
>> index 3a8d8df89186..10d0ce6c8342 100644
>> --- a/drivers/platform/x86/toshiba_acpi.c
>> +++ b/drivers/platform/x86/toshiba_acpi.c
>> @@ -3271,7 +3271,7 @@ static const char *find_hci_method(acpi_handle handle)
>>    */
>>   #define QUIRK_HCI_HOTKEY_QUICKSTART		BIT(1)
>>
>> -static const struct dmi_system_id toshiba_dmi_quirks[] = {
>> +static const struct dmi_system_id toshiba_dmi_quirks[] __initconst = {
>>   	{
>>   	 /* Toshiba Portégé R700 */
>>   	 /* https://bugzilla.kernel.org/show_bug.cgi?id=21012 */
>> @@ -3306,8 +3306,6 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>>   	struct toshiba_acpi_dev *dev;
>>   	const char *hci_method;
>>   	u32 dummy;
>> -	const struct dmi_system_id *dmi_id;
>> -	long quirks = 0;
>>   	int ret = 0;
>>
>>   	if (toshiba_acpi)
>> @@ -3460,16 +3458,6 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
>>   	}
>>   #endif
>>
>> -	dmi_id = dmi_first_match(toshiba_dmi_quirks);
>> -	if (dmi_id)
>> -		quirks = (long)dmi_id->driver_data;
>> -
>> -	if (turn_on_panel_on_resume == -1)
>> -		turn_on_panel_on_resume = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
>> -
>> -	if (hci_hotkey_quickstart == -1)
>> -		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);
>> -
>>   	toshiba_wwan_available(dev);
>>   	if (dev->wwan_supported)
>>   		toshiba_acpi_setup_wwan_rfkill(dev);
>> @@ -3618,10 +3606,27 @@ static struct acpi_driver toshiba_acpi_driver = {
>>   	.drv.pm	= &toshiba_acpi_pm,
>>   };
>>
>> +static void __init toshiba_dmi_init(void)
>> +{
>> +	const struct dmi_system_id *dmi_id;
>> +	long quirks = 0;
>> +
>> +	dmi_id = dmi_first_match(toshiba_dmi_quirks);
>> +	if (dmi_id)
>> +		quirks = (long)dmi_id->driver_data;
>> +
>> +	if (turn_on_panel_on_resume == -1)
>> +		turn_on_panel_on_resume = !!(quirks & QUIRK_TURN_ON_PANEL_ON_RESUME);
>> +
>> +	if (hci_hotkey_quickstart == -1)
>> +		hci_hotkey_quickstart = !!(quirks & QUIRK_HCI_HOTKEY_QUICKSTART);
>> +}
>> +
>>   static int __init toshiba_acpi_init(void)
>>   {
>>   	int ret;
>>
>> +	toshiba_dmi_init();
>>   	toshiba_proc_dir = proc_mkdir(PROC_TOSHIBA, acpi_root_dir);
>>   	if (!toshiba_proc_dir) {
>>   		pr_err("Unable to create proc dir " PROC_TOSHIBA "\n");
>> --
>> 2.39.2
>>