| 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
| ||
|
Message-ID: <79124489-0a2f-42ca-85ae-6f442e42e2d3@gmx.de>
Date: Sun, 18 May 2025 19:46:12 +0200
From: Armin Wolf <W_Armin@....de>
To: Janne Grunau <j@...nau.net>
Cc: Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>,
Vishnu Sankar <vishnuocv@...il.com>, "Rafael J. Wysocki"
<rafael@...nel.org>, Len Brown <lenb@...nel.org>,
linux-acpi@...r.kernel.org, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] HID: lenovo: Unbreak USB/BT keyboards on non-ACPI
platforms
Am 18.05.25 um 11:43 schrieb Janne Grunau:
> Hej,
>
> On Sat, May 17, 2025 at 05:58:24PM +0200, Armin Wolf wrote:
>> Am 16.05.25 um 01:05 schrieb Janne Grunau:
>>
>>> On Fri, May 16, 2025 at 12:05:11AM +0200, Armin Wolf wrote:
>>>> Am 12.05.25 um 23:55 schrieb Janne Grunau via B4 Relay:
>>>>
>>>>> From: Janne Grunau <j@...nau.net>
>>>>>
>>>>> Commit 84c9d2a968c8 ("HID: lenovo: Support for ThinkPad-X12-TAB-1/2 Kbd
>>>>> Fn keys") added a dependency on ACPI_PLATFORM_PROFILE to cycle through
>>>>> power profiles. This breaks USB and Bluetooth keyboards on non-ACPI
>>>>> platforms since platform_profile_init() fails. See the warning below for
>>>>> the visible symptom but cause is the dependency on the platform_profile
>>>>> module.
> [...]
>
>>>> i think we can fix that. We just have to skip the compat stuff if acpi_kobj is NULL (means that ACPI is not used).
>>>> The modern platform profile interface is generic enough to also work on non-ACPI systems.
>>>>
>>>> Can you test a patch?
>>> I can easily test patches
>> Nice, i attached the necessary patch. Please keep in mind that this patch is compile-tested only.
>>
>> From: Armin Wolf <W_Armin@....de>
>> Date: Sat, 17 May 2025 17:45:09 +0200
>> Subject: [PATCH] ACPI: platform_profile: Add support for non-ACPI platforms
>>
>> Currently the platform profile subsystem assumes that all supported
>> (i.e. ACPI-capable) platforms always run with ACPI being enabled.
>> However some ARM64 notebooks do not support ACPI and are instead
>> using devicetree for booting.
>>
>> Do not register the legacy sysfs interface on such devices as it
>> depends on the acpi_kobj (/sys/firmware/acpi/) being present. Users
>> are encouraged to use the new platform-profile class interface
>> instead.
>>
>> Signed-off-by: Armin Wolf <W_Armin@....de>
>> ---
>> drivers/acpi/platform_profile.c | 68 ++++++++++++++++++++++++++-------
>> 1 file changed, 55 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
>> index ffbfd32f4cf1..c5a5da7d03f1 100644
>> --- a/drivers/acpi/platform_profile.c
>> +++ b/drivers/acpi/platform_profile.c
>> @@ -190,6 +190,20 @@ static ssize_t profile_show(struct device *dev,
>> return sysfs_emit(buf, "%s\n", profile_names[profile]);
>> }
>>
>> +/**
>> + * profile_notify_legacy - Notify the legacy sysfs interface
>> + *
>> + * This wrapper takes care of only notifying the legacy sysfs interface
>> + * if it was registered during module initialization.
>> + */
>> +static void profile_notify_legacy(void)
>> +{
>> + if (!acpi_kobj)
>> + return;
>> +
>> + sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> +}
>> +
>> /**
>> * profile_store - Set the profile for a class device
>> * @dev: The device
>> @@ -215,7 +229,7 @@ static ssize_t profile_store(struct device *dev,
>> return ret;
>> }
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> + profile_notify_legacy();
>>
>> return count;
>> }
>> @@ -435,7 +449,7 @@ static ssize_t platform_profile_store(struct kobject *kobj,
>> return ret;
>> }
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> + profile_notify_legacy();
>>
>> return count;
>> }
>> @@ -472,6 +486,22 @@ static const struct attribute_group platform_profile_group = {
>> .is_visible = profile_class_is_visible,
>> };
>>
>> +/**
>> + * profile_update_legacy - Update the legacy sysfs interface
>> + *
>> + * This wrapper takes care of only updating the legacy sysfs interface
>> + * if it was registered during module initialization.
>> + *
>> + * Return: 0 on success or error code on failure.
>> + */
>> +static int profile_update_legacy(void)
>> +{
>> + if (!acpi_kobj)
>> + return 0;
>> +
>> + return sysfs_update_group(acpi_kobj, &platform_profile_group);
>> +}
>> +
>> /**
>> * platform_profile_notify - Notify class device and legacy sysfs interface
>> * @dev: The class device
>> @@ -481,7 +511,7 @@ void platform_profile_notify(struct device *dev)
>> scoped_cond_guard(mutex_intr, return, &profile_lock) {
>> _notify_class_profile(dev, NULL);
>> }
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> + profile_notify_legacy();
>> }
>> EXPORT_SYMBOL_GPL(platform_profile_notify);
>>
>> @@ -529,7 +559,7 @@ int platform_profile_cycle(void)
>> return err;
>> }
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> + profile_notify_legacy();
>>
>> return 0;
>> }
>> @@ -603,9 +633,9 @@ struct device *platform_profile_register(struct device *dev, const char *name,
>> goto cleanup_ida;
>> }
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> + profile_notify_legacy();
>>
>> - err = sysfs_update_group(acpi_kobj, &platform_profile_group);
>> + err = profile_update_legacy();
>> if (err)
>> goto cleanup_cur;
>>
>> @@ -639,8 +669,8 @@ void platform_profile_remove(struct device *dev)
>> ida_free(&platform_profile_ida, pprof->minor);
>> device_unregister(&pprof->dev);
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> - sysfs_update_group(acpi_kobj, &platform_profile_group);
>> + profile_notify_legacy();
>> + profile_update_legacy();
>> }
>> EXPORT_SYMBOL_GPL(platform_profile_remove);
>>
>> @@ -692,16 +722,28 @@ static int __init platform_profile_init(void)
>> if (err)
>> return err;
>>
>> - err = sysfs_create_group(acpi_kobj, &platform_profile_group);
>> - if (err)
>> - class_unregister(&platform_profile_class);
>> + /*
>> + * The ACPI kobject can be missing if ACPI was disabled during booting.
>> + * We thus skip the initialization of the legacy sysfs interface in such
>> + * cases to allow the platform profile class to work on ARM64 notebooks
>> + * without ACPI support.
> I wouldn't say work as I'd expect that there are 0 registered
> platform_profile class devices.
Just as expected, currently all drivers registering with the platform profile subsystem are
depending on ACPI. In the future this might change.
>> + */
>> + if (acpi_kobj) {
>> + err = sysfs_create_group(acpi_kobj, &platform_profile_group);
>> + if (err < 0) {
>> + class_unregister(&platform_profile_class);
>> + return err;
>> + }
>> + }
>>
>> - return err;
>> + return 0;
>> }
>>
>> static void __exit platform_profile_exit(void)
>> {
>> - sysfs_remove_group(acpi_kobj, &platform_profile_group);
>> + if (acpi_kobj)
>> + sysfs_remove_group(acpi_kobj, &platform_profile_group);
>> +
>> class_unregister(&platform_profile_class);
>> }
>> module_init(platform_profile_init);
> thanks, patch works on the affected system and the HID device for the
> Lenovo keyboard probes successfully. We still need to stub
> platform_profile_cycle() to get rid of the ACPI Kconfig dependency. I'll
> send that out separately.
>
> Reviewed-by: Janne Grunau <j@...nau.net>
> Tested-by: Janne Grunau <j@...nau.net>
>
Alright, i will send this patch to the ACPI mailing list ASAP. Please keep in mind
that merely stubbing out the affected functions is not enough, as the platform profile code
needs to be moved out of drivers/acpi/ as well.
Thanks,
Armin Wolf
Powered by blists - more mailing lists