1. Patchew
  2. linux
  3. View series

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

Thorsten Blum posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/platform/x86/lenovo/think-lmi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Thorsten Blum 1 month, 2 weeks ago 
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@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
Changes in v2:
- Keep and reword the newline comment
- v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
---
 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';
 	return count;
 }
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Ilpo Järvinen 1 month, 1 week ago 
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@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Keep and reword the newline comment
> - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> ---
>  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.
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Thorsten Blum 1 month, 1 week ago 
On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo Järvinen wrote:
> 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@vger.kernel.org
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> > Changes in v2:
> > - Keep and reword the newline comment
> > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> > ---
> >  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? 

It's not required, strscpy() would also work.

> Wouldn't it work with:
> 
> 	strscpy(..., pwdlen);
> 
> ?

However, strscpy(setting->password, buf, pwdlen) wouldn't work because
pwdlen is the number of characters to copy, but the destination buffer
size also needs to include room for the NUL terminator. Since there is
no NUL before pwdlen, it would copy only pwdlen - 1 characters and set
setting->password[pwdlen - 1] = '\0'.

strscpy(setting->password, buf, pwdlen + 1) would work, but since we
already know that exactly pwdlen bytes need to be copied, memcpy() is
sufficient.
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Ilpo Järvinen 1 month, 1 week ago 
On Tue, 18 Aug 2026, Thorsten Blum wrote:

> On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo Järvinen wrote:
> > 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@vger.kernel.org
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > ---
> > > Changes in v2:
> > > - Keep and reword the newline comment
> > > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> > > ---
> > >  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? 
> 
> It's not required, strscpy() would also work.
> 
> > Wouldn't it work with:
> > 
> > 	strscpy(..., pwdlen);
> > 
> > ?
> 
> However, strscpy(setting->password, buf, pwdlen) wouldn't work because
> pwdlen is the number of characters to copy, but the destination buffer
> size also needs to include room for the NUL terminator. Since there is
> no NUL before pwdlen, it would copy only pwdlen - 1 characters and set
> setting->password[pwdlen - 1] = '\0'.

Okay, I was thinking in my mind if I've a off-by-one error in my 
suggestion but didn't want to spend too much time on figuring it out.

> strscpy(setting->password, buf, pwdlen + 1) would work, but since we
> already know that exactly pwdlen bytes need to be copied, memcpy() is
> sufficient.

It may work, but since you then go to nul terminate it yourself, I think 
using the existing function is way better than memcpy() + custom nul 
termination code.

-- 
 i.
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Thorsten Blum 1 month, 1 week ago 
On Tue, Aug 18, 2026 at 04:20:10PM +0300, Ilpo Järvinen wrote:
> On Tue, 18 Aug 2026, Thorsten Blum wrote:
> > On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo Järvinen wrote:
> > > 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@vger.kernel.org
> > > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > > ---
> > > > Changes in v2:
> > > > - Keep and reword the newline comment
> > > > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> > > > ---
> > > >  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? 
> > 
> > It's not required, strscpy() would also work.
> > 
> > > Wouldn't it work with:
> > > 
> > > 	strscpy(..., pwdlen);
> > > 
> > > ?
> > 
> > However, strscpy(setting->password, buf, pwdlen) wouldn't work because
> > pwdlen is the number of characters to copy, but the destination buffer
> > size also needs to include room for the NUL terminator. Since there is
> > no NUL before pwdlen, it would copy only pwdlen - 1 characters and set
> > setting->password[pwdlen - 1] = '\0'.
> 
> Okay, I was thinking in my mind if I've a off-by-one error in my 
> suggestion but didn't want to spend too much time on figuring it out.
> 
> > strscpy(setting->password, buf, pwdlen + 1) would work, but since we
> > already know that exactly pwdlen bytes need to be copied, memcpy() is
> > sufficient.
> 
> It may work, but since you then go to nul terminate it yourself, I think 
> using the existing function is way better than memcpy() + custom nul 
> termination code.

I used memcpy() because we already determined the string length pwdlen,
which strscpy() would have to determine again internally.

The performance difference shouldn't matter here, so it's mostly a
matter of style or personal preference. I'm fine either way.

Thanks,
Thorsten
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by David Laight 1 month, 1 week ago 
On Tue, 18 Aug 2026 15:39:35 +0200
Thorsten Blum <thorsten.blum@linux.dev> wrote:

> On Tue, Aug 18, 2026 at 04:20:10PM +0300, Ilpo Järvinen wrote:
> > On Tue, 18 Aug 2026, Thorsten Blum wrote:  
> > > On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo Järvinen wrote:  
> > > > 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@vger.kernel.org
> > > > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > > > ---
> > > > > Changes in v2:
> > > > > - Keep and reword the newline comment
> > > > > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> > > > > ---
> > > > >  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?   
> > > 
> > > It's not required, strscpy() would also work.
> > >   
> > > > Wouldn't it work with:
> > > > 
> > > > 	strscpy(..., pwdlen);
> > > > 
> > > > ?  
> > > 
> > > However, strscpy(setting->password, buf, pwdlen) wouldn't work because
> > > pwdlen is the number of characters to copy, but the destination buffer
> > > size also needs to include room for the NUL terminator. Since there is
> > > no NUL before pwdlen, it would copy only pwdlen - 1 characters and set
> > > setting->password[pwdlen - 1] = '\0'.  
> > 
> > Okay, I was thinking in my mind if I've a off-by-one error in my 
> > suggestion but didn't want to spend too much time on figuring it out.
> >   
> > > strscpy(setting->password, buf, pwdlen + 1) would work, but since we
> > > already know that exactly pwdlen bytes need to be copied, memcpy() is
> > > sufficient.  

No that overwrites past the end out the output buffer.

> > 
> > It may work, but since you then go to nul terminate it yourself, I think 
> > using the existing function is way better than memcpy() + custom nul 
> > termination code.  
> 
> I used memcpy() because we already determined the string length pwdlen,
> which strscpy() would have to determine again internally.
> 
> The performance difference shouldn't matter here, so it's mostly a
> matter of style or personal preference. I'm fine either way.

Personally I'd always use memcpy() if the size is known.

Whoever added the strscpy() should have used the size of the array not
the soft bound for the password length (which is limited by the array size).

In any case the code could be:
	pwdlen = count;
	if (pwdlen && buf[pwdlen - 1] == '\n')
		pwdlen--;
	...
	memcpy(setting->password, buf, pwdlen);
	setting->password[pwdlen] = 0;
	return count;
}

I'm sure there could be (might even be) a helper to copy some
characters and append a '\0'.

David



> 
> Thanks,
> Thorsten
>
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Ilpo Järvinen 1 month, 1 week ago 
On Wed, 19 Aug 2026, David Laight wrote:

> On Tue, 18 Aug 2026 15:39:35 +0200
> Thorsten Blum <thorsten.blum@linux.dev> wrote:
> 
> > On Tue, Aug 18, 2026 at 04:20:10PM +0300, Ilpo Järvinen wrote:
> > > On Tue, 18 Aug 2026, Thorsten Blum wrote:  
> > > > On Tue, Aug 18, 2026 at 02:28:50PM +0300, Ilpo Järvinen wrote:  
> > > > > 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@vger.kernel.org
> > > > > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > - Keep and reword the newline comment
> > > > > > - v1: https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> > > > > > ---
> > > > > >  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?   
> > > > 
> > > > It's not required, strscpy() would also work.
> > > >   
> > > > > Wouldn't it work with:
> > > > > 
> > > > > 	strscpy(..., pwdlen);
> > > > > 
> > > > > ?  
> > > > 
> > > > However, strscpy(setting->password, buf, pwdlen) wouldn't work because
> > > > pwdlen is the number of characters to copy, but the destination buffer
> > > > size also needs to include room for the NUL terminator. Since there is
> > > > no NUL before pwdlen, it would copy only pwdlen - 1 characters and set
> > > > setting->password[pwdlen - 1] = '\0'.  
> > > 
> > > Okay, I was thinking in my mind if I've a off-by-one error in my 
> > > suggestion but didn't want to spend too much time on figuring it out.
> > >   
> > > > strscpy(setting->password, buf, pwdlen + 1) would work, but since we
> > > > already know that exactly pwdlen bytes need to be copied, memcpy() is
> > > > sufficient.  
> 
> No that overwrites past the end out the output buffer.

Hi David,

So are you suggesting ->password cannot hold a maxlen long password or 
what?

> > > It may work, but since you then go to nul terminate it yourself, I think 
> > > using the existing function is way better than memcpy() + custom nul 
> > > termination code.  
> > 
> > I used memcpy() because we already determined the string length pwdlen,
> > which strscpy() would have to determine again internally.
> > 
> > The performance difference shouldn't matter here, so it's mostly a
> > matter of style or personal preference. I'm fine either way.
> 
> Personally I'd always use memcpy() if the size is known.
> 
> Whoever added the strscpy() should have used the size of the array not
> the soft bound for the password length (which is limited by the array size).
> 
> In any case the code could be:
> 	pwdlen = count;
> 	if (pwdlen && buf[pwdlen - 1] == '\n')
> 		pwdlen--;
> 	...
> 	memcpy(setting->password, buf, pwdlen);
> 	setting->password[pwdlen] = 0;
> 	return count;
> }
> 
> I'm sure there could be (might even be) a helper to copy some
> characters and append a '\0'.

Are you perhaps looking for strscpy() ? :-)

--
 i.
Re: [PATCH v2] platform/x86: think-lmi: Fix current password length check
Posted by Mark Pearson 1 month, 2 weeks ago 
Thanks Thorsten

On Thu, Aug 13, 2026, at 4:20 AM, 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@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
> Changes in v2:
> - Keep and reword the newline comment
> - v1: 
> https://lore.kernel.org/r/20260810132018.156868-3-thorsten.blum@linux.dev/
> ---
>  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';
>  	return count;
>  }

Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Mark

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.