2023-01-11 06:41:09

by Yang Yang

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

From: Xu Panda <[email protected]>

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 <[email protected]>
Signed-off-by: Yang Yang <[email protected]>
---
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

2023-01-13 05:36:09

by Jakub Kicinski

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

On Wed, 11 Jan 2023 14:25:48 +0800 (CST) [email protected] wrote:
> From: Xu Panda <[email protected]>
>
> 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));

2023-01-13 07:25:25

by Yang Yang

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

> 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.

2023-01-13 19:46:02

by Jakub Kicinski

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

On Fri, 13 Jan 2023 15:13:12 +0800 (CST) [email protected] 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.