| 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: <3cbbecf7-bba4-4a45-97da-ec461609b2f9@amd.com>
Date: Wed, 6 Nov 2024 15:25:44 -0600
From: Mario Limonciello <mario.limonciello@....com>
To: Armin Wolf <W_Armin@....de>, Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Maximilian Luz <luzmaximilian@...il.com>, Lee Chun-Yi <jlee@...e.com>,
Shyam Sundar S K <Shyam-sundar.S-k@....com>,
Corentin Chary <corentin.chary@...il.com>, "Luke D . Jones"
<luke@...nes.dev>, Ike Panhc <ike.pan@...onical.com>,
Henrique de Moraes Holschuh <hmh@....eng.br>,
Alexis Belmonte <alexbelm48@...il.com>,
Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Ai Chao <aichao@...inos.cn>, Gergo Koteles <soyer@....hu>,
open list <linux-kernel@...r.kernel.org>,
"open list:ACPI" <linux-acpi@...r.kernel.org>,
"open list:MICROSOFT SURFACE PLATFORM PROFILE DRIVER"
<platform-driver-x86@...r.kernel.org>,
"open list:THINKPAD ACPI EXTRAS DRIVER"
<ibm-acpi-devel@...ts.sourceforge.net>,
Mark Pearson <mpearson-lenovo@...ebb.ca>,
Matthew Schwartz <matthew.schwartz@...ux.dev>
Subject: Re: [PATCH v4 16/20] ACPI: platform_profile: Make sure all profile
handlers agree on profile
On 11/6/2024 14:58, Armin Wolf wrote:
> Am 05.11.24 um 16:33 schrieb Mario Limonciello:
>
>> If for any reason multiple profile handlers don't agree on the profile
>> return the custom profile.
>>
>> Reviewed-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>> drivers/acpi/platform_profile.c | 84 +++++++++++++++++++++------------
>> 1 file changed, 53 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/
>> platform_profile.c
>> index d8c2d195106c2..7861fccc2e58c 100644
>> --- a/drivers/acpi/platform_profile.c
>> +++ b/drivers/acpi/platform_profile.c
>> @@ -280,55 +280,77 @@ static ssize_t
>> platform_profile_choices_show(struct device *dev,
>> return _commmon_choices_show(aggregate, buf);
>> }
>>
>> -static ssize_t platform_profile_show(struct device *dev,
>> - struct device_attribute *attr,
>> - char *buf)
>> +/**
>> + * _aggregate_profiles - Aggregate the profiles for legacy sysfs
>> interface
>> + * @dev: The device
>> + * @data: The profile to return
>> + * Return: 0 on success, -errno on failure
>> + */
>> +static int _aggregate_profiles(struct device *dev, void *data)
>> {
>> - enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
>> + enum platform_profile_option *profile = data;
>> + enum platform_profile_option val;
>> int err;
>>
>> - scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
>> - if (!cur_profile)
>> - return -ENODEV;
>> + err = get_class_profile(dev, &val);
>> + if (err)
>> + return err;
>>
>> - err = cur_profile->profile_get(cur_profile, &profile);
>> - if (err)
>> - return err;
>> - }
>> + if (*profile != PLATFORM_PROFILE_LAST && *profile != val)
>> + *profile = PLATFORM_PROFILE_CUSTOM;
>> + else
>> + *profile = val;
>>
>> - /* Check that profile is valid index */
>> - if (WARN_ON((profile < 0) || (profile >=
>> ARRAY_SIZE(profile_names))))
>> - return -EIO;
>
> Please check the returned value from profile_get() first before doing
> the custom platform
> profile check.
You might have missed it - that's part of get_class_profile() already.
>
>> + return 0;
>> +}
>> +
>> +/**
>> + * platform_profile_show - Show the current profile for legacy sysfs
>> interface
>> + * @dev: The device
>> + * @attr: The attribute
>> + * @buf: The buffer to write to
>> + * Return: The number of bytes written
>> + */
>> +static ssize_t platform_profile_show(struct device *dev,
>> + struct device_attribute *attr,
>> + char *buf)
>> +{
>> + enum platform_profile_option profile = PLATFORM_PROFILE_LAST;
>> + int err;
>> +
>> + err = class_for_each_device(&platform_profile_class, NULL,
>> + &profile, _aggregate_profiles);
>
> Missing error handling.
>
Right, got it.
>>
>> return sysfs_emit(buf, "%s\n", profile_names[profile]);
>> }
>>
>> +/**
>> + * platform_profile_store - Set the profile for legacy sysfs interface
>> + * @dev: The device
>> + * @attr: The attribute
>> + * @buf: The buffer to read from
>> + * @count: The number of bytes to read
>> + * Return: The number of bytes read
>> + */
>> static ssize_t platform_profile_store(struct device *dev,
>> - struct device_attribute *attr,
>> - const char *buf, size_t count)
>> + struct device_attribute *attr,
>> + const char *buf, size_t count)
>> {
>> - int err, i;
>> + int ret;
>> + int i;
>>
>> /* Scan for a matching profile */
>> i = sysfs_match_string(profile_names, buf);
>> if (i < 0)
>> return -EINVAL;
>> -
>> - scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &profile_lock) {
>> - if (!cur_profile)
>> - return -ENODEV;
>> -
>> - /* Check that platform supports this profile choice */
>> - if (!test_bit(i, cur_profile->choices))
>> - return -EOPNOTSUPP;
>> -
>> - err = cur_profile->profile_set(cur_profile, i);
>> - if (err)
>> - return err;
>> + if (i == PLATFORM_PROFILE_CUSTOM) {
>> + pr_warn("Custom profile not supported for legacy sysfs
>> interface\n");
>
> This would allow userspace applications to spam the kernel log. Please
> just return -EINVAL here
> and document this special case inside the interface specification.
>
OK.
> Thanks,
> Armin Wolf
>
>> + return -EINVAL;
>> }
>>
>> - sysfs_notify(acpi_kobj, NULL, "platform_profile");
>> - return count;
>> + ret = class_for_each_device(&platform_profile_class, NULL, &i,
>> _store_class_profile);
>> +
>> + return ret ? ret : count;
>> }
>>
>> static DEVICE_ATTR_RO(platform_profile_choices);
Powered by blists - more mailing lists