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

[Armin Wolf]

Am 16.01.24 um 01:12 schrieb Sasha Levin:

> From: Armin Wolf <W_Armin@xxxxxx>
>
> [ 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@xxxxxxxxxx>
> Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
> Link: https://lore.kernel.org/r/20231218192420.305411-3-W_Armin@xxxxxx
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
>   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)