diff mbox series

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

Message ID 202301111425483027624@zte.com.cn
State New
Headers
Series [net-next,v2] net/rds: use strscpy() to instead of strncpy() |

Commit Message

Yang Yang Jan. 11, 2023, 6:25 a.m. UTC 
  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(-)

Comments

Jakub Kicinski Jan. 13, 2023, 5:17 a.m. UTC | #1 
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));
Yang Yang Jan. 13, 2023, 7:13 a.m. UTC | #2 
> 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.
Jakub Kicinski Jan. 13, 2023, 7:28 p.m. UTC | #3 
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.
diff mbox series

Patch

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));