[PATCH] net: rds: Replace strncpy with strscpy in connection setup

[PATCH] net: rds: Replace strncpy with strscpy in connection setup

[Shankari02]

This patch replaces strncpy() with strscpy(), which is the
preferred, safer alternative. strscpy() guarantees null-termination
as long as the destination buffer is non-zero in size, and also
provides a return value that can be used to detect truncation.

This change is made in accordance with the Linux kernel
documentation which marks strncpy() as deprecated for bounded
string copying:

  https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..fb2f14a1279a 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

Re: [PATCH] net: rds: Replace strncpy with strscpy in connection setup

[Jakub Kicinski]

On Wed, 23 Apr 2025 21:07:30 +0530 Shankari02 wrote:
> This patch replaces strncpy() with strscpy(), which is the
> preferred, safer alternative. strscpy() guarantees null-termination
> as long as the destination buffer is non-zero in size, and also
> provides a return value that can be used to detect truncation.
> 
> This change is made in accordance with the Linux kernel
> documentation which marks strncpy() as deprecated for bounded
> string copying:

You need to explain why padding is not necessary.

Please use full name in the Author / From line, like you used in the
Sign-off tag.
-- 
pw-bot: cr

[PATCH v2] net: rds: Replace strncpy with strscpy in connection setup

[Shankari Anand]

From: Shankari02 <shankari.ak0208@gmail.com>

This patch replaces strncpy() with strscpy(), which is the preferred, safer
alternative for bounded string copying in the Linux kernel. strscpy() guarantees
null-termination as long as the destination buffer is non-zero in size and provides
a return value to detect truncation.

Padding of the 'transport' field is not necessary because it is treated purely
as a null-terminated string and is not used for binary comparisons or direct
memory operations that would rely on padding. Therefore, switching to strscpy()
is safe and appropriate here.

This change is made in accordance with the Linux kernel documentation, which
marks strncpy() as deprecated for bounded string operations:
https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..fb2f14a1279a 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

Re: [PATCH v2] net: rds: Replace strncpy with strscpy in connection setup

[Shankari]

Hi Jacub,

I have implemented the changes in the v2 patch. Thanks for your review.

On Sun, Apr 27, 2025 at 12:51 AM Shankari Anand
<shankari.ak0208@gmail.com> wrote:
>
> From: Shankari02 <shankari.ak0208@gmail.com>
>
> This patch replaces strncpy() with strscpy(), which is the preferred, safer
> alternative for bounded string copying in the Linux kernel. strscpy() guarantees
> null-termination as long as the destination buffer is non-zero in size and provides
> a return value to detect truncation.
>
> Padding of the 'transport' field is not necessary because it is treated purely
> as a null-terminated string and is not used for binary comparisons or direct
> memory operations that would rely on padding. Therefore, switching to strscpy()
> is safe and appropriate here.
>
> This change is made in accordance with the Linux kernel documentation, which
> marks strncpy() as deprecated for bounded string operations:
> https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy
>
> Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
> ---
>  net/rds/connection.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..fb2f14a1279a 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>         cinfo->laddr = conn->c_laddr.s6_addr32[3];
>         cinfo->faddr = conn->c_faddr.s6_addr32[3];
>         cinfo->tos = conn->c_tos;
> -       strncpy(cinfo->transport, conn->c_trans->t_name,
> +       strscpy(cinfo->transport, conn->c_trans->t_name,
>                 sizeof(cinfo->transport));
>         cinfo->flags = 0;
>
> @@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>         cinfo6->next_rx_seq = cp->cp_next_rx_seq;
>         cinfo6->laddr = conn->c_laddr;
>         cinfo6->faddr = conn->c_faddr;
> -       strncpy(cinfo6->transport, conn->c_trans->t_name,
> +       strscpy(cinfo6->transport, conn->c_trans->t_name,
>                 sizeof(cinfo6->transport));
>         cinfo6->flags = 0;
>
> --
> 2.34.1
>

Re: [PATCH v2] net: rds: Replace strncpy with strscpy in connection setup

[Allison Henderson]

On Sun, 2025-04-27 at 00:51 +0530, Shankari Anand wrote:
> From: Shankari02 <shankari.ak0208@gmail.com>
> 
> This patch replaces strncpy() with strscpy(), which is the preferred, safer
> alternative for bounded string copying in the Linux kernel. strscpy() guarantees
> null-termination as long as the destination buffer is non-zero in size and provides
> a return value to detect truncation.
> 
> Padding of the 'transport' field is not necessary because it is treated purely
> as a null-terminated string and is not used for binary comparisons or direct
> memory operations that would rely on padding. Therefore, switching to strscpy()
> is safe and appropriate here.
> 
> This change is made in accordance with the Linux kernel documentation, which
> marks strncpy() as deprecated for bounded string operations:
> https://urldefense.com/v3/__https://www.kernel.org/doc/html/latest/process/deprecated.html*strcpy__;Iw!!ACWV5N9M2RV99hQ!JQ3FhnKPxlGvQkqNxOtHRJfVNmGFTbZV2VX0GApjASrhCXFfe0I_W27l08BCIavcbHWaiWZBbOA8JeMeVieuPuUgll4OXE0$ 
> 
> Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
This looks fine to me.  Thanks Shankari!

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>  net/rds/connection.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..fb2f14a1279a 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>  	cinfo->laddr = conn->c_laddr.s6_addr32[3];
>  	cinfo->faddr = conn->c_faddr.s6_addr32[3];
>  	cinfo->tos = conn->c_tos;
> -	strncpy(cinfo->transport, conn->c_trans->t_name,
> +	strscpy(cinfo->transport, conn->c_trans->t_name,
>  		sizeof(cinfo->transport));
>  	cinfo->flags = 0;
>  
> @@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>  	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
>  	cinfo6->laddr = conn->c_laddr;
>  	cinfo6->faddr = conn->c_faddr;
> -	strncpy(cinfo6->transport, conn->c_trans->t_name,
> +	strscpy(cinfo6->transport, conn->c_trans->t_name,
>  		sizeof(cinfo6->transport));
>  	cinfo6->flags = 0;
>  

Re: [PATCH v2] net: rds: Replace strncpy with strscpy in connection setup

[Jakub Kicinski]

On Sun, 27 Apr 2025 00:51:13 +0530 Shankari Anand wrote:
> From: Shankari02 <shankari.ak0208@gmail.com>

Not your full name, the last From: in the email is what matters.
Maybe try to correct the author of the commit in your git tree.

> This patch replaces strncpy() with strscpy(), which is the preferred, safer
> alternative for bounded string copying in the Linux kernel. strscpy() guarantees
> null-termination as long as the destination buffer is non-zero in size and provides
> a return value to detect truncation.

Please wrap the commit message at 72 chars.

> Padding of the 'transport' field is not necessary because it is treated purely
> as a null-terminated string and is not used for binary comparisons or direct
> memory operations that would rely on padding. Therefore, switching to strscpy()
> is safe and appropriate here.

It's treated as purely null-terminated string? Where exactly
in the code do you see that? Because all I see is a memcpy..

Re: [PATCH v2] net: rds: Replace strncpy with strscpy in connection setup

[Simon Horman]

On Sun, Apr 27, 2025 at 12:56:14AM +0530, Shankari wrote:
> Hi Jacub,
> 
> I have implemented the changes in the v2 patch. Thanks for your review.
> 
> On Sun, Apr 27, 2025 at 12:51 AM Shankari Anand
> <shankari.ak0208@gmail.com> wrote:
> >
> > From: Shankari02 <shankari.ak0208@gmail.com>
> >
> > This patch replaces strncpy() with strscpy(), which is the preferred, safer
> > alternative for bounded string copying in the Linux kernel. strscpy() guarantees
> > null-termination as long as the destination buffer is non-zero in size and provides
> > a return value to detect truncation.
> >
> > Padding of the 'transport' field is not necessary because it is treated purely
> > as a null-terminated string and is not used for binary comparisons or direct
> > memory operations that would rely on padding. Therefore, switching to strscpy()
> > is safe and appropriate here.
> >
> > This change is made in accordance with the Linux kernel documentation, which
> > marks strncpy() as deprecated for bounded string operations:
> > https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy
> >
> > Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
> > ---
> >  net/rds/connection.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/rds/connection.c b/net/rds/connection.c
> > index c749c5525b40..fb2f14a1279a 100644
> > --- a/net/rds/connection.c
> > +++ b/net/rds/connection.c
> > @@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
> >         cinfo->laddr = conn->c_laddr.s6_addr32[3];
> >         cinfo->faddr = conn->c_faddr.s6_addr32[3];
> >         cinfo->tos = conn->c_tos;
> > -       strncpy(cinfo->transport, conn->c_trans->t_name,
> > +       strscpy(cinfo->transport, conn->c_trans->t_name,
> >                 sizeof(cinfo->transport));

Because cinfo->transport is an array it's length is sizeof(cinfo->transport),
as used above. But for the same reason the two-argument version of
strscpy() can be used here.

	strscpy(cinfo->transport, conn->c_trans->t_name);

Similarly if, based on my understanding of Jakub's feedback,
strscpy_pad() should be used.

> >         cinfo->flags = 0;
> >
> > @@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
> >         cinfo6->next_rx_seq = cp->cp_next_rx_seq;
> >         cinfo6->laddr = conn->c_laddr;
> >         cinfo6->faddr = conn->c_faddr;
> > -       strncpy(cinfo6->transport, conn->c_trans->t_name,
> > +       strscpy(cinfo6->transport, conn->c_trans->t_name,
> >                 sizeof(cinfo6->transport));

Ditto.

> >         cinfo6->flags = 0;
> >
> > --
> > 2.34.1
> >
> 

[PATCH v3] net: rds: Replace strncpy with strscpy in connection setup

[Shankari Anand]

From: Shankari02 <shankari.ak0208@gmail.com>

Replaces strncpy() with strscpy_pad() for copying the transport field.
Unlike strscpy(), strscpy_pad() ensures the destination buffer is fully padded with null bytes, avoiding garbage data.
This is safer for struct copies and comparisons. As strncpy() is deprecated (see: kernel.org/doc/html/latest/process/deprecated.html#strcpy),
this change improves correctness and adheres to kernel guidelines for safe, bounded string handling.

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..fb2f14a1279a 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

[PATCH v3] net: rds: Replace strncpy with strscpy in connection setup

[Shankari Anand]

From: Shankari02 <shankari.ak0208@gmail.com>

Replaces strncpy() with strscpy_pad() for copying the transport field.
Unlike strscpy(), strscpy_pad() ensures the destination buffer is fully padded with null bytes, avoiding garbage data.
This is safer for struct copies and comparisons. As strncpy() is deprecated (see: kernel.org/doc/html/latest/process/deprecated.html#strcpy),
this change improves correctness and adheres to kernel guidelines for safe, bounded string handling.

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..fb2f14a1279a 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

[PATCH v3] net: rds: Replace strncpy with strscpy in connection setup

[Shankari Anand]

Replaces strncpy() with strscpy_pad() for copying the transport field.
Unlike strscpy(), strscpy_pad() ensures the destination buffer is fully padded with null bytes, avoiding garbage data.
This is safer for struct copies and comparisons. As strncpy() is deprecated (see: kernel.org/doc/html/latest/process/deprecated.html#strcpy),
this change improves correctness and adheres to kernel guidelines for safe, bounded string handling.

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..fb2f14a1279a 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

[PATCH v4] net: rds: Replace strncpy with strscpy in connection setup

[Shankari Anand]

Replaces strncpy() with strscpy_pad() for copying the transport field.
Unlike strscpy(), strscpy_pad() ensures the destination buffer is fully padded with null bytes, avoiding garbage data.
This is safer for struct copies and comparisons. As strncpy() is deprecated (see: kernel.org/doc/html/latest/process/deprecated.html#strcpy),
this change improves correctness and adheres to kernel guidelines for safe, bounded string handling.

Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
---
 net/rds/connection.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..4689062db84f 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo->laddr = conn->c_laddr.s6_addr32[3];
 	cinfo->faddr = conn->c_faddr.s6_addr32[3];
 	cinfo->tos = conn->c_tos;
-	strncpy(cinfo->transport, conn->c_trans->t_name,
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name,
 		sizeof(cinfo->transport));
 	cinfo->flags = 0;
 
@@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
 	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
 	cinfo6->laddr = conn->c_laddr;
 	cinfo6->faddr = conn->c_faddr;
-	strncpy(cinfo6->transport, conn->c_trans->t_name,
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name,
 		sizeof(cinfo6->transport));
 	cinfo6->flags = 0;
 
-- 
2.34.1

Re: [PATCH v4] net: rds: Replace strncpy with strscpy in connection setup

[Allison Henderson]

On Wed, 2025-05-21 at 18:28 +0530, Shankari Anand wrote:
> Replaces strncpy() with strscpy_pad() for copying the transport field.
> Unlike strscpy(), strscpy_pad() ensures the destination buffer is fully padded with null bytes, avoiding garbage data.
> This is safer for struct copies and comparisons. As strncpy() is deprecated (see: kernel.org/doc/html/latest/process/deprecated.html#strcpy),
> this change improves correctness and adheres to kernel guidelines for safe, bounded string handling.
> 
> Signed-off-by: Shankari Anand <shankari.ak0208@gmail.com>
Hi Shankari,

Thanks for the patch, but it looks like Baris already submitted a similar patch, and I believe the two argument variant
is correct for this use case since cinfo->transport is a fixed sized array.

Thank you for your patch submission though!
Allison

> ---
>  net/rds/connection.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..4689062db84f 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,7 +749,7 @@ static int rds_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>  	cinfo->laddr = conn->c_laddr.s6_addr32[3];
>  	cinfo->faddr = conn->c_faddr.s6_addr32[3];
>  	cinfo->tos = conn->c_tos;
> -	strncpy(cinfo->transport, conn->c_trans->t_name,
> +	strscpy_pad(cinfo->transport, conn->c_trans->t_name,
>  		sizeof(cinfo->transport));
>  	cinfo->flags = 0;
>  
> @@ -775,7 +775,7 @@ static int rds6_conn_info_visitor(struct rds_conn_path *cp, void *buffer)
>  	cinfo6->next_rx_seq = cp->cp_next_rx_seq;
>  	cinfo6->laddr = conn->c_laddr;
>  	cinfo6->faddr = conn->c_faddr;
> -	strncpy(cinfo6->transport, conn->c_trans->t_name,
> +	strscpy_pad(cinfo6->transport, conn->c_trans->t_name,
>  		sizeof(cinfo6->transport));
>  	cinfo6->flags = 0;
>