[PATCH net-next v2] net/rds: use strscpy() to instead of strncpy()

[PATCH net-next v2] net/rds: use strscpy() to instead of strncpy()

[yang.yang29]

From: Xu Panda <xu.panda@zte.com.cn>

The implementation of strscpy() is more robust and safer.
That's now the recommended way to copy NUL-terminated strings.

Signed-off-by: Xu Panda <xu.panda@zte.com.cn>
Signed-off-by: Yang Yang <yang.yang29@zte.com.cn>
---
change for v2
 - Use the returns of strscpy to make the copy and the preceding 
BUG_ON() together.Thanks to Jakub Kicinski. 
---
 net/rds/stats.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/rds/stats.c b/net/rds/stats.c
index 9e87da43c004..7018c67418f5 100644
--- a/net/rds/stats.c
+++ b/net/rds/stats.c
@@ -88,9 +88,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
 	size_t i;

 	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';
+		BUG_ON(strscpy(ctr.name, names[i], sizeof(ctr.name)) < 0);
 		ctr.value = values[i];

 		rds_info_copy(iter, &ctr, sizeof(ctr));
-- 
2.15.2

Re: [PATCH net-next v2] net/rds: use strscpy() to instead of strncpy()

[Jakub Kicinski]

On Wed, 11 Jan 2023 14:25:48 +0800 (CST) yang.yang29@zte.com.cn wrote:
> From: Xu Panda <xu.panda@zte.com.cn>
> 
> The implementation of strscpy() is more robust and safer.
> That's now the recommended way to copy NUL-terminated strings.

What are the differences in behavior between strncpy() and strscpy()?

> diff --git a/net/rds/stats.c b/net/rds/stats.c
> index 9e87da43c004..7018c67418f5 100644
> --- a/net/rds/stats.c
> +++ b/net/rds/stats.c
> @@ -88,9 +88,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
>  	size_t i;
> 
>  	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';
> +		BUG_ON(strscpy(ctr.name, names[i], sizeof(ctr.name)) < 0);
>  		ctr.value = values[i];
> 
>  		rds_info_copy(iter, &ctr, sizeof(ctr));

Re: [PATCH net-next v2] net/rds: use strscpy() to instead of strncpy()

[yang.yang29]

> What are the differences in behavior between strncpy() and strscpy()?

Strscpy() makes the dest string NUL-terminated, and returns more
useful value. While strncpy() can initialize the dest string.

Here we use strscpy() to make dest string NUL-terminated, and use
return value to check src string size and dest string size. This make
the code simpler.

Re: [PATCH net-next v2] net/rds: use strscpy() to instead of strncpy()

[Jakub Kicinski]

On Fri, 13 Jan 2023 15:13:12 +0800 (CST) yang.yang29@zte.com.cn wrote:
> > What are the differences in behavior between strncpy() and strscpy()?  
> 
> Strscpy() makes the dest string NUL-terminated, and returns more
> useful value. While strncpy() can initialize the dest string.
> 
> Here we use strscpy() to make dest string NUL-terminated, and use
> return value to check src string size and dest string size. This make
> the code simpler.

I'm not sure whether in this particular case the output needs 
to be padded or not. And I'm not sure you understand what the
implications are.

The code is fine as is, and I don't trust that you know what 
you're doing. So please don't send any more strncpy() -> strscpy()
conversions for networking.

If you want to do something useful please start with adding a check 
to checkpatch to warn people against using strncpy() and suggest using
strscpy() instead.