linux-kernel - Re: [PATCH 3/4] platform/x86: think-lmi: Allow empty admin password

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <7b3de78f-fa9f-4213-b9c2-3dfb9c7f3e45@app.fastmail.com>
Date: Tue, 22 Oct 2024 15:19:23 -0400
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: "Hans de Goede" <hdegoede@...hat.com>,
 "platform-driver-x86@...r.kernel.org" <platform-driver-x86@...r.kernel.org>,
 LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] platform/x86: think-lmi: Allow empty admin password

Hi Ilpo

On Tue, Oct 22, 2024, at 3:59 AM, Ilpo Järvinen wrote:
> On Mon, 21 Oct 2024, Mark Pearson wrote:
>
>> SVP = BIOS Supervisor/Admin password
>> SMP = BIOS System password
>> 
>> If SMP ACL is enabled in the BIOS then the system allows you to set the
>> SMP without a SVP password configured. Change code to allow this.
>> BIOS will return permissions error if SVP is required.
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@...ebb.ca>
>> ---
>>  drivers/platform/x86/think-lmi.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
>> index 46ab82fb2898..751e351dfc42 100644
>> --- a/drivers/platform/x86/think-lmi.c
>> +++ b/drivers/platform/x86/think-lmi.c
>> @@ -469,7 +469,12 @@ static ssize_t new_password_store(struct kobject *kobj,
>>  		if (ret)
>>  			goto out;
>>  
>> -		if (tlmi_priv.pwd_admin->pwd_enabled) {
>> +		/*
>> +		 * Note admin password not always required if SMPControl enabled in BIOS,
>
> is not always required
>
>> +		 * So only set if it's configured.
>> +		 * Let BIOS figure it out - we'll get an error if operation not permitted
>
> operation is not permitted.
>
> Please reflow the comment to fit into 80 chars (this only relates to 
> comments to keep their line lengths readable without large eye movement).
>
>> +		 */
>> +		if (tlmi_priv.pwd_admin->pwd_enabled && strlen(tlmi_priv.pwd_admin->password)) {
>>  			ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin",
>>  					tlmi_priv.pwd_admin->password);
>>  			if (ret)
>> 
>
> -- 
>  i.

Thanks for the review.
I'll update those. I'll give it a couple of days in case there is any other feedback
Mark