Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check

[Ilpo Järvinen]

On Thu, 13 Aug 2026, Thorsten Blum wrote:

> current_password_store() checks the password length before removing the
> trailing newline, which can reject valid passwords that are exactly
> ->maxlen bytes long.
> 
> It also passes ->maxlen to strscpy(), which truncates passwords without
> a newline.
> 
> Use strchrnul() to measure the password length up to the newline, then
> copy that many bytes and add a trailing NUL terminator.
> 
> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxx>
> ---
> Changes in v2:
> - Keep and reword the newline comment
> - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@xxxxxxxxx/
> ---
>  drivers/platform/x86/lenovo/think-lmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/lenovo/think-lmi.c b/drivers/platform/x86/lenovo/think-lmi.c
> index e215e86e3db7..d0ceb6aaa69e 100644
> --- a/drivers/platform/x86/lenovo/think-lmi.c
> +++ b/drivers/platform/x86/lenovo/think-lmi.c
> @@ -438,14 +438,14 @@ static ssize_t current_password_store(struct kobject *kobj,
>  	struct tlmi_pwd_setting *setting = to_tlmi_pwd_setting(kobj);
>  	size_t pwdlen;
>  
> -	pwdlen = strlen(buf);
> +	/* Strip newline; setting password won't work if one is present. */
> +	pwdlen = strchrnul(buf, '\n') - buf;
>  	/* pwdlen == 0 is allowed to clear the password */
>  	if (pwdlen && ((pwdlen < setting->minlen) || (pwdlen > setting->maxlen)))
>  		return -EINVAL;
>  
> -	strscpy(setting->password, buf, setting->maxlen);
> -	/* Strip out CR if one is present, setting password won't work if it is present */
> -	strreplace(setting->password, '\n', '\0');
> +	memcpy(setting->password, buf, pwdlen);
> +	setting->password[pwdlen] = '\0';

Hi,

I don't understand why is this strscpy() -> memcpy() conversion required? 

Wouldn't it work with:

	strscpy(..., pwdlen);

?

strscpy() forces NUL termination and there shouldn't be any NULs or 
newlines before pwdlen.

-- 
 i.