[PATCH] platform/x86: think-lmi: Keep pending save state on failure

[PATCH] platform/x86: think-lmi: Keep pending save state on failure

[Thorsten Blum]

In save_settings_store(), the opcode and legacy password paths do not
check if saving BIOS settings failed, but instead unconditionally clear
save_required and set reboot_required, losing the pending save state.

Check the save result in those paths too.

Fixes: 318d97849fc2 ("platform/x86: think-lmi: Add bulk save feature")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/platform/x86/lenovo/think-lmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
index e215e86e3db7..cc0a85d1b832 100644
--- a/drivers/platform/x86/lenovo/think-lmi.c
+++ b/drivers/platform/x86/lenovo/think-lmi.c
@@ -1347,6 +1347,8 @@ static ssize_t save_settings_store(struct kobject *kobj, struct kobj_attribute *
 					goto out;
 			}
 			ret = tlmi_save_bios_settings("");
+			if (ret)
+				goto out;
 		} else { /* old non-opcode based authentication method (deprecated) */
 			if (tlmi_priv.pwd_admin->pwd_enabled && tlmi_priv.pwd_admin->password[0]) {
 				auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
@@ -1363,6 +1365,8 @@ static ssize_t save_settings_store(struct kobject *kobj, struct kobj_attribute *
 				ret = tlmi_save_bios_settings(auth_str);
 			else
 				ret = tlmi_save_bios_settings("");
+			if (ret)
+				goto out;
 		}
 		tlmi_priv.save_required = false;
 		tlmi_priv.reboot_required = true;

Re: [PATCH] platform/x86: think-lmi: Keep pending save state on failure

[Mark Pearson]

Hi Thorsten,

On Mon, Aug 10, 2026, at 8:08 AM, Thorsten Blum wrote:
> In save_settings_store(), the opcode and legacy password paths do not
> check if saving BIOS settings failed, but instead unconditionally clear
> save_required and set reboot_required, losing the pending save state.
>
> Check the save result in those paths too.
>
> Fixes: 318d97849fc2 ("platform/x86: think-lmi: Add bulk save feature")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/platform/x86/lenovo/think-lmi.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/platform/x86/lenovo/think-lmi.c 
> b/drivers/platform/x86/lenovo/think-lmi.c
> index e215e86e3db7..cc0a85d1b832 100644
> --- a/drivers/platform/x86/lenovo/think-lmi.c
> +++ b/drivers/platform/x86/lenovo/think-lmi.c
> @@ -1347,6 +1347,8 @@ static ssize_t save_settings_store(struct kobject 
> *kobj, struct kobj_attribute *
>  					goto out;
>  			}
>  			ret = tlmi_save_bios_settings("");
> +			if (ret)
> +				goto out;
>  		} else { /* old non-opcode based authentication method (deprecated) 
> */
>  			if (tlmi_priv.pwd_admin->pwd_enabled && 
> tlmi_priv.pwd_admin->password[0]) {
>  				auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
> @@ -1363,6 +1365,8 @@ static ssize_t save_settings_store(struct kobject 
> *kobj, struct kobj_attribute *
>  				ret = tlmi_save_bios_settings(auth_str);
>  			else
>  				ret = tlmi_save_bios_settings("");
> +			if (ret)
> +				goto out;
>  		}
>  		tlmi_priv.save_required = false;
>  		tlmi_priv.reboot_required = true;

Curious - have you actually hit this condition?

My main thought is that if the save fails - chances are that something is pretty messed up and doing another save isn't going to fix it. Most likely you're going to have to reboot.

I'll see if I can get an answer from the FW team on expectations - but getting straight answers for error paths without a repro mechanism can be tricky.

Mark

Re: [PATCH] platform/x86: think-lmi: Keep pending save state on failure

[Thorsten Blum]

On Tue, Aug 11, 2026 at 03:29:16PM -0400, Mark Pearson wrote:
> Hi Thorsten,
> 
> On Mon, Aug 10, 2026, at 8:08 AM, Thorsten Blum wrote:
> > In save_settings_store(), the opcode and legacy password paths do not
> > check if saving BIOS settings failed, but instead unconditionally clear
> > save_required and set reboot_required, losing the pending save state.
> >
> > Check the save result in those paths too.
> >
> > Fixes: 318d97849fc2 ("platform/x86: think-lmi: Add bulk save feature")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  drivers/platform/x86/lenovo/think-lmi.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/platform/x86/lenovo/think-lmi.c 
> > b/drivers/platform/x86/lenovo/think-lmi.c
> > index e215e86e3db7..cc0a85d1b832 100644
> > --- a/drivers/platform/x86/lenovo/think-lmi.c
> > +++ b/drivers/platform/x86/lenovo/think-lmi.c
> > @@ -1347,6 +1347,8 @@ static ssize_t save_settings_store(struct kobject 
> > *kobj, struct kobj_attribute *
> >  					goto out;
> >  			}
> >  			ret = tlmi_save_bios_settings("");
> > +			if (ret)
> > +				goto out;
> >  		} else { /* old non-opcode based authentication method (deprecated) 
> > */
> >  			if (tlmi_priv.pwd_admin->pwd_enabled && 
> > tlmi_priv.pwd_admin->password[0]) {
> >  				auth_str = kasprintf(GFP_KERNEL, "%s,%s,%s;",
> > @@ -1363,6 +1365,8 @@ static ssize_t save_settings_store(struct kobject 
> > *kobj, struct kobj_attribute *
> >  				ret = tlmi_save_bios_settings(auth_str);
> >  			else
> >  				ret = tlmi_save_bios_settings("");
> > +			if (ret)
> > +				goto out;
> >  		}
> >  		tlmi_priv.save_required = false;
> >  		tlmi_priv.reboot_required = true;
> 
> Curious - have you actually hit this condition?

No, I was just reading the code and wondered why those return values
aren't checked for errors.