| 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: <a601983d-2a49-4119-8e94-bde0f1e949d5@app.fastmail.com>
Date: Fri, 22 Aug 2025 12:15:04 -0400
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: "Hans de Goede" <hansg@...nel.org>, RenHai <kean0048@...il.com>,
"platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] platform/x86: think-lmi: Add certificate GUID structure
Thanks Ilpo,
On Fri, Aug 22, 2025, at 11:41 AM, Ilpo Järvinen wrote:
> On Fri, 22 Aug 2025, Mark Pearson wrote:
>
>> Add a certificate GUID structure to make it easier to add different
>> options for other platforms that need different GUIDs.
>>
>> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
>> Signed-off-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
>> ---
>> drivers/platform/x86/lenovo/think-lmi.c | 41 ++++++++++++++++++++-----
>> 1 file changed, 33 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
>> index 0992b41b6221..88bae5b33c57 100644
>> --- a/drivers/platform/x86/lenovo/think-lmi.c
>> +++ b/drivers/platform/x86/lenovo/think-lmi.c
>> @@ -177,6 +177,28 @@ MODULE_PARM_DESC(debug_support, "Enable debug command support");
>> #define TLMI_CERT_SVC BIT(7) /* Admin Certificate Based */
>> #define TLMI_CERT_SMC BIT(8) /* System Certificate Based */
>>
>> +struct tlmi_cert_guids {
>> + char *thumbprint;
>> + char *set_bios_setting;
>> + char *save_bios_setting;
>> + char *cert_to_password;
>> + char *clear_bios_cert;
>> + char *update_bios_cert;
>> + char *set_bios_cert;
>> +};
>> +
>> +static struct tlmi_cert_guids thinkpad_cert_guid = {
>> + LENOVO_CERT_THUMBPRINT_GUID,
>
> Don't use the anonymous initialization but name the members for better
> readability.
>
Sure - I will fix that
>> + LENOVO_SET_BIOS_SETTING_CERT_GUID,
>> + LENOVO_SAVE_BIOS_SETTING_CERT_GUID,
>> + LENOVO_CERT_TO_PASSWORD_GUID,
>> + LENOVO_CLEAR_BIOS_CERT_GUID,
>> + LENOVO_UPDATE_BIOS_CERT_GUID,
>> + LENOVO_SET_BIOS_CERT_GUID
>
> Always remember to add comma to any non-terminator entry.
>
Yep - will do. Interesting that the checkpatch --strict didn't catch that one.
> ...And thanks for reworking these, they look so simple now to review. :-)
>
It was a good suggestion :)
>> +};
>> +
>> +static struct tlmi_cert_guids *cert_guid = &thinkpad_cert_guid;
>> +
>> static const struct tlmi_err_codes tlmi_errs[] = {
>> {"Success", 0},
>> {"Not Supported", -EOPNOTSUPP},
>> @@ -668,7 +690,10 @@ static ssize_t cert_thumbprint(char *buf, const char *arg, int count)
>> const union acpi_object *obj;
>> acpi_status status;
>>
>> - status = wmi_evaluate_method(LENOVO_CERT_THUMBPRINT_GUID, 0, 0, &input, &output);
>> + if (!cert_guid->thumbprint)
>> + return -EOPNOTSUPP;
>
> Either mention this in the changelog or move it into the next patch
> that is the one needing the check. The latter of those seems more logical
> as this is the only GUID you're NULL checking.
>
I did debate whether it was patch 1 or 2.
I think you're right...I'll move it to patch 2
> --
> i.
>
>> +
>> + status = wmi_evaluate_method(cert_guid->thumbprint, 0, 0, &input, &output);
>> if (ACPI_FAILURE(status)) {
>> kfree(output.pointer);
>> return -EIO;
>> @@ -751,7 +776,7 @@ static ssize_t cert_to_password_store(struct kobject *kobj,
>> kfree_sensitive(passwd);
>> return -ENOMEM;
>> }
>> - ret = tlmi_simple_call(LENOVO_CERT_TO_PASSWORD_GUID, auth_str);
>> + ret = tlmi_simple_call(cert_guid->cert_to_password, auth_str);
>> kfree(auth_str);
>> kfree_sensitive(passwd);
>>
>> @@ -797,7 +822,7 @@ static ssize_t certificate_store(struct kobject *kobj,
>> if (!auth_str)
>> return -ENOMEM;
>>
>> - ret = tlmi_simple_call(LENOVO_CLEAR_BIOS_CERT_GUID, auth_str);
>> + ret = tlmi_simple_call(cert_guid->clear_bios_cert, auth_str);
>> kfree(auth_str);
>>
>> return ret ?: count;
>> @@ -834,7 +859,7 @@ static ssize_t certificate_store(struct kobject *kobj,
>> kfree(new_cert);
>> return -EACCES;
>> }
>> - guid = LENOVO_UPDATE_BIOS_CERT_GUID;
>> + guid = cert_guid->update_bios_cert;
>> /* Format: 'Certificate,Signature' */
>> auth_str = cert_command(setting, new_cert, signature);
>> } else {
>> @@ -845,7 +870,7 @@ static ssize_t certificate_store(struct kobject *kobj,
>> kfree(new_cert);
>> return -EACCES;
>> }
>> - guid = LENOVO_SET_BIOS_CERT_GUID;
>> + guid = cert_guid->set_bios_cert;
>> /* Format: 'Certificate, password' */
>> auth_str = cert_command(setting, new_cert, setting->password);
>> }
>> @@ -1071,13 +1096,13 @@ static ssize_t current_value_store(struct kobject *kobj,
>> goto out;
>> }
>>
>> - ret = tlmi_simple_call(LENOVO_SET_BIOS_SETTING_CERT_GUID, set_str);
>> + ret = tlmi_simple_call(cert_guid->set_bios_setting, set_str);
>> if (ret)
>> goto out;
>> if (tlmi_priv.save_mode == TLMI_SAVE_BULK)
>> tlmi_priv.save_required = true;
>> else
>> - ret = tlmi_simple_call(LENOVO_SAVE_BIOS_SETTING_CERT_GUID,
>> + ret = tlmi_simple_call(cert_guid->save_bios_setting,
>> tlmi_priv.pwd_admin->save_signature);
>> } else if (tlmi_priv.opcode_support) {
>> /*
>> @@ -1282,7 +1307,7 @@ static ssize_t save_settings_store(struct kobject *kobj, struct kobj_attribute *
>> ret = -EINVAL;
>> goto out;
>> }
>> - ret = tlmi_simple_call(LENOVO_SAVE_BIOS_SETTING_CERT_GUID,
>> + ret = tlmi_simple_call(cert_guid->save_bios_setting,
>> tlmi_priv.pwd_admin->save_signature);
>> if (ret)
>> goto out;
>>
Powered by blists - more mailing lists