linux-kernel - Re: [PATCH AUTOSEL 6.7 15/18] platform/x86: wmi: Remove ACPI handlers after WMI devices

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0d179914-5912-46ee-9c10-50b9bbcac718@gmx.de>
Date: Tue, 16 Jan 2024 12:53:46 +0100
From: Armin Wolf <W_Armin@....de>
To: Sasha Levin <sashal@...nel.org>, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Cc: Hans de Goede <hdegoede@...hat.com>, ilpo.jarvinen@...ux.intel.com,
 platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 6.7 15/18] platform/x86: wmi: Remove ACPI handlers
 after WMI devices

Am 16.01.24 um 01:12 schrieb Sasha Levin:

> From: Armin Wolf <W_Armin@....de>
>
> [ Upstream commit 22574e17626391ad969af9a13aaa58a1b37ad384 ]
>
> When removing the ACPI notify/address space handlers, the WMI devices
> are still active and might still depend on ACPI EC access or
> WMI events.
> Fix this by removing the ACPI handlers after all WMI devices
> associated with an ACPI device have been removed.

Hello,

i would advise against including this patch in the next stable kernels.

The WMI ACPI notify handler is still using list_for_each_entry() which is
not safe when the WMI devices are removed before the ACPI notify handler.

This issue existed inside the WMI code for a long time, but this patch might
trigger it. Since no users reported any issues regarding ACPI errors during
WMI removal, i would drop this patch.

Thanks,
Armin Wolf

> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
> Signed-off-by: Armin Wolf <W_Armin@....de>
> Link: https://lore.kernel.org/r/20231218192420.305411-3-W_Armin@gmx.de
> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>   drivers/platform/x86/wmi.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
> index 5dd22258cb3b..d400e61d6801 100644
> --- a/drivers/platform/x86/wmi.c
> +++ b/drivers/platform/x86/wmi.c
> @@ -1494,13 +1494,12 @@ static void acpi_wmi_remove(struct platform_device *device)
>   	struct acpi_device *acpi_device = ACPI_COMPANION(&device->dev);
>   	struct device *wmi_bus_device = dev_get_drvdata(&device->dev);
>
> -	acpi_remove_notify_handler(acpi_device->handle, ACPI_ALL_NOTIFY,
> -				   acpi_wmi_notify_handler);
> -	acpi_remove_address_space_handler(acpi_device->handle,
> -				ACPI_ADR_SPACE_EC, &acpi_wmi_ec_space_handler);
> -
>   	device_for_each_child_reverse(wmi_bus_device, NULL, wmi_remove_device);
>   	device_unregister(wmi_bus_device);
> +
> +	acpi_remove_notify_handler(acpi_device->handle, ACPI_ALL_NOTIFY, acpi_wmi_notify_handler);
> +	acpi_remove_address_space_handler(acpi_device->handle, ACPI_ADR_SPACE_EC,
> +					  &acpi_wmi_ec_space_handler);
>   }
>
>   static int acpi_wmi_probe(struct platform_device *device)