| Message ID | 20240209152359.528919-1-mpearson-lenovo@squebb.ca (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Hans de Goede |
| Headers | show |
| Series | platform/x86: think-lmi: Fix password opcode ordering for workstations | expand |
On Fri, 9 Feb 2024, Mark Pearson wrote: > The Lenovo workstations require the password opcode to be run before > the attribute value is changed (if Admin password is enabled). > > Tested on some Thinkpads to confirm they are OK with this order too. > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> Would a Fixes tag be appropriate here?
Thanks Ilpo On Fri, Feb 9, 2024, at 10:34 AM, Ilpo Järvinen wrote: > On Fri, 9 Feb 2024, Mark Pearson wrote: > >> The Lenovo workstations require the password opcode to be run before >> the attribute value is changed (if Admin password is enabled). >> >> Tested on some Thinkpads to confirm they are OK with this order too. >> >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > > Would a Fixes tag be appropriate here? Hmmm - good point, though it has been "wrong" since it was originally implemented (where we tested on Thinkpads). I guess I could use the commit ID from when I originally implemented opcodes? Fixes: 640a5fa ("platform/x86: think-lmi: Opcode support") Do you want me to push a new version with this? Thanks Mark
On Fri, 9 Feb 2024, Mark Pearson wrote: > Thanks Ilpo > > On Fri, Feb 9, 2024, at 10:34 AM, Ilpo Järvinen wrote: > > On Fri, 9 Feb 2024, Mark Pearson wrote: > > > >> The Lenovo workstations require the password opcode to be run before > >> the attribute value is changed (if Admin password is enabled). > >> > >> Tested on some Thinkpads to confirm they are OK with this order too. > >> > >> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> > > > > Would a Fixes tag be appropriate here? > > Hmmm - good point, though it has been "wrong" since it was originally implemented (where we tested on Thinkpads). > > I guess I could use the commit ID from when I originally implemented opcodes? > Fixes: 640a5fa ("platform/x86: think-lmi: Opcode support") Yes, in that case the original commit is the correct one. > Do you want me to push a new version with this? I think this is simple enough to go through fixes branch so it depends on Hans. Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Hi, On 2/9/24 16:23, Mark Pearson wrote: > The Lenovo workstations require the password opcode to be run before > the attribute value is changed (if Admin password is enabled). > > Tested on some Thinkpads to confirm they are OK with this order too. > > Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> Thank you for your patch, I've applied this patch to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans I've also added the Fixes tag and Ilpo's Reviewed-by. Note it will show up in my review-hans branch once I've pushed my local branch there, which might take a while. I will include this patch in my next fixes pull-req to Linus for the current kernel development cycle. Regards, Hans > --- > drivers/platform/x86/think-lmi.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c > index 3a396b763c49..ce3e08815a8e 100644 > --- a/drivers/platform/x86/think-lmi.c > +++ b/drivers/platform/x86/think-lmi.c > @@ -1009,7 +1009,16 @@ static ssize_t current_value_store(struct kobject *kobj, > * Note - this sets the variable and then the password as separate > * WMI calls. Function tlmi_save_bios_settings will error if the > * password is incorrect. > + * Workstation's require the opcode to be set before changing the > + * attribute. > */ > + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", > + tlmi_priv.pwd_admin->password); > + if (ret) > + goto out; > + } > + > set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, > new_setting); > if (!set_str) { > @@ -1021,17 +1030,10 @@ static ssize_t current_value_store(struct kobject *kobj, > if (ret) > goto out; > > - if (tlmi_priv.save_mode == TLMI_SAVE_BULK) { > + if (tlmi_priv.save_mode == TLMI_SAVE_BULK) > tlmi_priv.save_required = true; > - } else { > - if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > - ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", > - tlmi_priv.pwd_admin->password); > - if (ret) > - goto out; > - } > + else > ret = tlmi_save_bios_settings(""); > - } > } else { /* old non-opcode based authentication method (deprecated) */ > if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { > auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c index 3a396b763c49..ce3e08815a8e 100644 --- a/drivers/platform/x86/think-lmi.c +++ b/drivers/platform/x86/think-lmi.c @@ -1009,7 +1009,16 @@ static ssize_t current_value_store(struct kobject *kobj, * Note - this sets the variable and then the password as separate * WMI calls. Function tlmi_save_bios_settings will error if the * password is incorrect. + * Workstation's require the opcode to be set before changing the + * attribute. */ + if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { + ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", + tlmi_priv.pwd_admin->password); + if (ret) + goto out; + } + set_str = kasprintf(GFP_KERNEL, "%s,%s;", setting->display_name, new_setting); if (!set_str) { @@ -1021,17 +1030,10 @@ static ssize_t current_value_store(struct kobject *kobj, if (ret) goto out; - if (tlmi_priv.save_mode == TLMI_SAVE_BULK) { + if (tlmi_priv.save_mode == TLMI_SAVE_BULK) tlmi_priv.save_required = true; - } else { - if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { - ret = tlmi_opcode_setting("WmiOpcodePasswordAdmin", - tlmi_priv.pwd_admin->password); - if (ret) - goto out; - } + else ret = tlmi_save_bios_settings(""); - } } else { /* old non-opcode based authentication method (deprecated) */ if (tlmi_priv.pwd_admin->valid && tlmi_priv.pwd_admin->password[0]) { auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
The Lenovo workstations require the password opcode to be run before the attribute value is changed (if Admin password is enabled). Tested on some Thinkpads to confirm they are OK with this order too. Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca> --- drivers/platform/x86/think-lmi.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)