1. Patchew
  2. linux
  3. View series

[PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()

Thorsten Blum posted 1 patch 10 months ago 
net/rds/stats.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Thorsten Blum 10 months ago 
strncpy() is deprecated for NUL-terminated destination buffers. Use
strscpy_pad() instead and remove the manual NUL-termination.

Compile-tested only.

Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 net/rds/stats.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/rds/stats.c b/net/rds/stats.c
index 9e87da43c004..cb2e3d2cdf73 100644
--- a/net/rds/stats.c
+++ b/net/rds/stats.c
@@ -89,8 +89,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
 
 	for (i = 0; i < nr; i++) {
 		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
-		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
-		ctr.name[sizeof(ctr.name) - 1] = '\0';
+		strscpy_pad(ctr.name, names[i]);
 		ctr.value = values[i];
 
 		rds_info_copy(iter, &ctr, sizeof(ctr));
-- 
2.48.1
Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Allison Henderson 10 months ago 
On Wed, 2025-02-19 at 23:47 +0100, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers. Use
> strscpy_pad() instead and remove the manual NUL-termination.
> 
> Compile-tested only.
> 
> Link: https://urldefense.com/v3/__https://github.com/KSPP/linux/issues/90__;!!ACWV5N9M2RV99hQ!MpqMAmj6IIyu7Vj4ddfEGJlJY4rVrJL_g8etOQsHC7pdjZO77P7aOqJe8_JTFwBzZ6tciUDrbb2CjXWJMjdEMJGtpoeBfHU8qw$ 
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  net/rds/stats.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/rds/stats.c b/net/rds/stats.c
> index 9e87da43c004..cb2e3d2cdf73 100644
> --- a/net/rds/stats.c
> +++ b/net/rds/stats.c
> @@ -89,8 +89,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
>  
>  	for (i = 0; i < nr; i++) {
>  		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
> -		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
> -		ctr.name[sizeof(ctr.name) - 1] = '\0';
> +		strscpy_pad(ctr.name, names[i]);
>  		ctr.value = values[i];
>  
Looks ok to me.  Thanks Thorsten!
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

>  		rds_info_copy(iter, &ctr, sizeof(ctr));
Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Allison Henderson 10 months ago 
On Thu, 2025-02-20 at 11:07 -0700, Allison Henderson wrote:
> On Wed, 2025-02-19 at 23:47 +0100, Thorsten Blum wrote:
> > strncpy() is deprecated for NUL-terminated destination buffers. Use
> > strscpy_pad() instead and remove the manual NUL-termination.
> > 
> > Compile-tested only.

I went ahead and tested this for you, and it looks fine.  So, you can go ahead and remove the "Compiled-tested only" and
add:
Tested-by: Allison Henderson <allison.henderson@oracle.com>

Thank you!
Allison
> > 
> > Link: https://urldefense.com/v3/__https://github.com/KSPP/linux/issues/90__;!!ACWV5N9M2RV99hQ!MpqMAmj6IIyu7Vj4ddfEGJlJY4rVrJL_g8etOQsHC7pdjZO77P7aOqJe8_JTFwBzZ6tciUDrbb2CjXWJMjdEMJGtpoeBfHU8qw$ 
> > Cc: linux-hardening@vger.kernel.org
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  net/rds/stats.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/net/rds/stats.c b/net/rds/stats.c
> > index 9e87da43c004..cb2e3d2cdf73 100644
> > --- a/net/rds/stats.c
> > +++ b/net/rds/stats.c
> > @@ -89,8 +89,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
> >  
> >  	for (i = 0; i < nr; i++) {
> >  		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
> > -		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
> > -		ctr.name[sizeof(ctr.name) - 1] = '\0';
> > +		strscpy_pad(ctr.name, names[i]);
> >  		ctr.value = values[i];
> >  
> Looks ok to me.  Thanks Thorsten!
> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> 
> >  		rds_info_copy(iter, &ctr, sizeof(ctr));
>
Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Kees Cook 10 months ago 
On Wed, Feb 19, 2025 at 11:47:31PM +0100, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers. Use
> strscpy_pad() instead and remove the manual NUL-termination.

When doing these conversions, please describe two aspects of
conversions:

- Why is it safe to be NUL terminated
- Why is it safe to be/not-be NUL-padded

In this case, the latter needs examination. Looking at how ctr is used,
it is memcpy()ed later, which means this string MUST be NUL padded or it
will leak stack memory contents.

So, please use strscpy_pad() here. :)

-Kees

> 
> Compile-tested only.
> 
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  net/rds/stats.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/rds/stats.c b/net/rds/stats.c
> index 9e87da43c004..cb2e3d2cdf73 100644
> --- a/net/rds/stats.c
> +++ b/net/rds/stats.c
> @@ -89,8 +89,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
>  
>  	for (i = 0; i < nr; i++) {
>  		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
> -		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
> -		ctr.name[sizeof(ctr.name) - 1] = '\0';
> +		strscpy_pad(ctr.name, names[i]);
>  		ctr.value = values[i];
>  
>  		rds_info_copy(iter, &ctr, sizeof(ctr));
> -- 
> 2.48.1
> 
> 

-- 
Kees Cook
Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Thorsten Blum 10 months ago 
On 20. Feb 2025, at 03:57, Kees Cook wrote:
> On Wed, Feb 19, 2025 at 11:47:31PM +0100, Thorsten Blum wrote:
>> strncpy() is deprecated for NUL-terminated destination buffers. Use
>> strscpy_pad() instead and remove the manual NUL-termination.
> 
> When doing these conversions, please describe two aspects of
> conversions:
> 
> - Why is it safe to be NUL terminated
> - Why is it safe to be/not-be NUL-padded
> 
> In this case, the latter needs examination. Looking at how ctr is used,
> it is memcpy()ed later, which means this string MUST be NUL padded or it
> will leak stack memory contents.
> 
> So, please use strscpy_pad() here. :)

I am using strscpy_pad() here already because of the NUL-padding.

Did you just miss that?

Thanks,
Thorsten
Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy_pad()
Posted by Kees Cook 10 months ago 

On February 19, 2025 11:04:18 PM PST, Thorsten Blum <thorsten.blum@linux.dev> wrote:
>On 20. Feb 2025, at 03:57, Kees Cook wrote:
>> On Wed, Feb 19, 2025 at 11:47:31PM +0100, Thorsten Blum wrote:
>>> strncpy() is deprecated for NUL-terminated destination buffers. Use
>>> strscpy_pad() instead and remove the manual NUL-termination.
>> 
>> When doing these conversions, please describe two aspects of
>> conversions:
>> 
>> - Why is it safe to be NUL terminated
>> - Why is it safe to be/not-be NUL-padded
>> 
>> In this case, the latter needs examination. Looking at how ctr is used,
>> it is memcpy()ed later, which means this string MUST be NUL padded or it
>> will leak stack memory contents.
>> 
>> So, please use strscpy_pad() here. :)
>
>I am using strscpy_pad() here already because of the NUL-padding.
>
>Did you just miss that?

Well that's embarrassing. Yes, I must need stronger glasses. *sigh* Apologies for the noise!

Reviewed-by: Kees Cook <kees@kernel.org>


-- 
Kees Cook

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.