public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] net: rds transform strncpy to strscpy
       [not found] <20250407183052.8763-1-goralbaris@gmail.com>
@ 2025-04-08 18:22 ` Simon Horman
  2025-04-08 21:21   ` [PATCH net-next v2] Replace strncpy with strscpy Baris Can Goral
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Horman @ 2025-04-08 18:22 UTC (permalink / raw)
  To: Baris Can Goral
  Cc: davem, edumazet, kuba, pabeni, netdev, allison.henderson, skhan,
	linux-rdma

+ linux-rdma@vger.kernel.org

Hi Baris,

On Mon, Apr 07, 2025 at 09:30:53PM +0300, Baris Can Goral wrote:
> Hi,

It's nice to be friendly, but I don't think a salutation
belongs in a commit message. (Please remove the line above.)

> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string,resulting in potential memory

Space after the comma (,) please.

> content exposures, unbounded reads, or crashes.

I think there should be a blank like before the Link tag.

> Link:https://github.com/KSPP/linux/issues/90

But not between it and other tags.

Also, there should be a space after "Link:"

Link: https://github.com/KSPP/linux/issues/90
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
>  net/rds/connection.c | 4 ++--
>  net/rds/stats.c      | 2 +-
>  2 files changed, 3 insertions(+), 3 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));

I agree that strscpy() is appropriate as we want null termination
but not padding.

Because the destination, cinfo->transport, is an array I believe
we can omit passing the size argument to strscpy, like this:

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

Link: https://docs.kernel.org/core-api/kernel-api.html#c.strscpy

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

Ditto.

>  
> diff --git a/net/rds/stats.c b/net/rds/stats.c
> index 9e87da43c004..63c34dbdf97f 100644
> --- a/net/rds/stats.c
> +++ b/net/rds/stats.c
> @@ -89,7 +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);
> +		strscpy(ctr.name, names[i], sizeof(ctr.name) - 1);
>  		ctr.name[sizeof(ctr.name) - 1] = '\0';
>  		ctr.value = values[i];

This issue appears to have been addressed by
commit c451715d78e3 ("net/rds: Replace deprecated strncpy() with strscpy_pad()")

As a Networking patch please make sure it is based on the net-next tree and
targeted at that tree like this:

  Subject: [PATCH net-next v2] ...

Or the net tree if it is a big fix, which this patch isn't.

Please consider posting a v2 patch to address the above.
And please consider CCing linux-rdma@vger.kernel.org on v2.

-- 
pw-bot: changes-requested

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH net-next v2] Replace strncpy with strscpy
  2025-04-08 18:22 ` [PATCH] net: rds transform strncpy to strscpy Simon Horman
@ 2025-04-08 21:21   ` Baris Can Goral
  2025-05-17 18:12     ` goralbaris
  0 siblings, 1 reply; 13+ messages in thread
From: Baris Can Goral @ 2025-04-08 21:21 UTC (permalink / raw)
  To: allison.henderson, davem, edumazet, kuba, pabeni
  Cc: horms, linux-rdma, skhan, Baris Can Goral

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string, resulting in potential memory.
Link: https://github.com/KSPP/linux/issues/90

Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
 net/rds/connection.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..4bb727dd13b0 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,8 +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,
-		sizeof(cinfo->transport));
+	strscpy(cinfo->transport, conn->c_trans->t_name);
 	cinfo->flags = 0;
 
 	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
@@ -775,7 +774,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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH net-next v2] Replace strncpy with strscpy
  2025-04-08 21:21   ` [PATCH net-next v2] Replace strncpy with strscpy Baris Can Goral
@ 2025-05-17 18:12     ` goralbaris
  2025-05-18  9:00       ` Simon Horman
  0 siblings, 1 reply; 13+ messages in thread
From: goralbaris @ 2025-05-17 18:12 UTC (permalink / raw)
  To: allison.henderson, davem, edumazet, kuba, pabeni; +Cc: horms, linux-rdma, skhan

Hi,
Any news about this Patch?

Best Regards,
Baris

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH net-next v2] Replace strncpy with strscpy
  2025-05-17 18:12     ` goralbaris
@ 2025-05-18  9:00       ` Simon Horman
  2025-05-18 19:53         ` [PATCH v3 net-next: rds] replace strncpy with strscpy_pad goralbaris
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Horman @ 2025-05-18  9:00 UTC (permalink / raw)
  To: goralbaris
  Cc: allison.henderson, davem, edumazet, kuba, pabeni, linux-rdma,
	skhan

On Sat, May 17, 2025 at 09:12:48PM +0300, goralbaris wrote:
> Hi,
> Any news about this Patch?

Hi Baris,

I expect this patch floundered because it was not CCed to netdev.

In any case, there was a separate effort to address this problem.

- [PATCH v2] net: rds: Replace strncpy with strscpy in connection setup
  by Shankari Anand
  https://lore.kernel.org/netdev/20250426192113.47012-1-shankari.ak0208@gmail.com/

I would suggest that a successful patch should:

* Use the two-argument variant of strscpy_pad (see thread above)
* Include commentary in the commit message regarding
  - Why strctpy* is preferred over strncpy (you already did that :).
  - Why strscpy_pad is appropriate instead of strscpy
* CC all relevant parties, including netdev, the Netdev maintainers,
  Shankari Anand and Allison Henderson

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 net-next: rds] replace strncpy with strscpy_pad
  2025-05-18  9:00       ` Simon Horman
@ 2025-05-18 19:53         ` goralbaris
  2025-05-19  7:01           ` Michal Swiatkowski
  0 siblings, 1 reply; 13+ messages in thread
From: goralbaris @ 2025-05-18 19:53 UTC (permalink / raw)
  To: horms
  Cc: allison.henderson, davem, edumazet, goralbaris, kuba, linux-rdma,
	pabeni, skhan, shankari.ak0208, netdev

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string, resulting in potential memory.
Link: https://github.com/KSPP/linux/issues/90

In addition, strscpy_pad is more appropriate because it also zero-fills 
any remaining space in the destination if the source is shorter than 
the provided buffer size.

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

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..d62f486ab29f 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,8 +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,
-		sizeof(cinfo->transport));
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
 	cinfo->flags = 0;
 
 	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
@@ -775,8 +774,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,
-		sizeof(cinfo6->transport));
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
 	cinfo6->flags = 0;
 
 	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 net-next: rds] replace strncpy with strscpy_pad
  2025-05-18 19:53         ` [PATCH v3 net-next: rds] replace strncpy with strscpy_pad goralbaris
@ 2025-05-19  7:01           ` Michal Swiatkowski
  2025-05-19 12:51             ` [PATCH v4 " Baris Can Goral
  0 siblings, 1 reply; 13+ messages in thread
From: Michal Swiatkowski @ 2025-05-19  7:01 UTC (permalink / raw)
  To: goralbaris
  Cc: horms, allison.henderson, davem, edumazet, kuba, linux-rdma,
	pabeni, skhan, shankari.ak0208, netdev

On Sun, May 18, 2025 at 10:53:29PM +0300, goralbaris wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string, resulting in potential memory.
> Link: https://github.com/KSPP/linux/issues/90
> 
> In addition, strscpy_pad is more appropriate because it also zero-fills 
> any remaining space in the destination if the source is shorter than 
> the provided buffer size.
> 
> Signed-off-by: goralbaris <goralbaris@gmail.com>

There should be your full name, not nick.

Feel free to add my RB tag
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> ---
>  net/rds/connection.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..d62f486ab29f 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,8 +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,
> -		sizeof(cinfo->transport));
> +	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
>  	cinfo->flags = 0;
>  
>  	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
> @@ -775,8 +774,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,
> -		sizeof(cinfo6->transport));
> +	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
>  	cinfo6->flags = 0;
>  
>  	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
> -- 
> 2.34.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4 net-next: rds] replace strncpy with strscpy_pad
  2025-05-19  7:01           ` Michal Swiatkowski
@ 2025-05-19 12:51             ` Baris Can Goral
  2025-05-19 21:15               ` Allison Henderson
  0 siblings, 1 reply; 13+ messages in thread
From: Baris Can Goral @ 2025-05-19 12:51 UTC (permalink / raw)
  To: michal.swiatkowski
  Cc: allison.henderson, davem, edumazet, goralbaris, horms, kuba,
	linux-rdma, netdev, pabeni, shankari.ak0208, skhan

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string, resulting in potential memory.
Link: https://github.com/KSPP/linux/issues/90

In addition, strscpy_pad is more appropriate because it also zero-fills
any remaining space in the destination if the source is shorter than
the provided buffer size.

Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
 net/rds/connection.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..d62f486ab29f 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,8 +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,
-		sizeof(cinfo->transport));
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
 	cinfo->flags = 0;
 
 	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
@@ -775,8 +774,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,
-		sizeof(cinfo6->transport));
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
 	cinfo6->flags = 0;
 
 	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v4 net-next: rds] replace strncpy with strscpy_pad
  2025-05-19 12:51             ` [PATCH v4 " Baris Can Goral
@ 2025-05-19 21:15               ` Allison Henderson
  2025-05-20 16:23                 ` [PATCH v5 " Baris Can Goral
  0 siblings, 1 reply; 13+ messages in thread
From: Allison Henderson @ 2025-05-19 21:15 UTC (permalink / raw)
  To: michal.swiatkowski@linux.intel.com, goralbaris@gmail.com
  Cc: linux-rdma@vger.kernel.org, davem@davemloft.net,
	shankari.ak0208@gmail.com, pabeni@redhat.com, horms@kernel.org,
	edumazet@google.com, kuba@kernel.org, skhan@linuxfoundation.org,
	netdev@vger.kernel.org

On Mon, 2025-05-19 at 15:51 +0300, Baris Can Goral wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string, resulting in potential memory.
It looks like we lost the last part of this phrase? I think you meant to quote the link: "...potential memory content
exposures, unbounded reads, or crashes."

Other than that I think it looks ok.  Thanks!
Allison

> Link: https://urldefense.com/v3/__https://github.com/KSPP/linux/issues/90__;!!ACWV5N9M2RV99hQ!Ja5aVj9u5vDpeBsiMWIGFvGhVzCbcj-gUOS9qIbQ_QDVAy_GU9E4yl_yCjzYJ61uEfuo368zv8bY5vsmB9yxR-7h$ 
> 
> In addition, strscpy_pad is more appropriate because it also zero-fills
> any remaining space in the destination if the source is shorter than
> the provided buffer size.
> 
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
>  net/rds/connection.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..d62f486ab29f 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,8 +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,
> -		sizeof(cinfo->transport));
> +	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
>  	cinfo->flags = 0;
>  
>  	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
> @@ -775,8 +774,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,
> -		sizeof(cinfo6->transport));
> +	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
>  	cinfo6->flags = 0;
>  
>  	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 net-next: rds] replace strncpy with strscpy_pad
  2025-05-19 21:15               ` Allison Henderson
@ 2025-05-20 16:23                 ` Baris Can Goral
  2025-05-20 21:13                   ` Zhu Yanjun
  0 siblings, 1 reply; 13+ messages in thread
From: Baris Can Goral @ 2025-05-20 16:23 UTC (permalink / raw)
  To: allison.henderson
  Cc: davem, edumazet, goralbaris, horms, kuba, linux-rdma,
	michal.swiatkowski, netdev, pabeni, shankari.ak0208, skhan

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string, resulting in potential memory
content exposures, unbounded reads, or crashes.
Link: https://github.com/KSPP/linux/issues/90

In addition, strscpy_pad is more appropriate because it also zero-fills
any remaining space in the destination if the source is shorter than
the provided buffer size.

Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
 net/rds/connection.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..d62f486ab29f 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,8 +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,
-		sizeof(cinfo->transport));
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
 	cinfo->flags = 0;
 
 	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
@@ -775,8 +774,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,
-		sizeof(cinfo6->transport));
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
 	cinfo6->flags = 0;
 
 	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next: rds] replace strncpy with strscpy_pad
  2025-05-20 16:23                 ` [PATCH v5 " Baris Can Goral
@ 2025-05-20 21:13                   ` Zhu Yanjun
  0 siblings, 0 replies; 13+ messages in thread
From: Zhu Yanjun @ 2025-05-20 21:13 UTC (permalink / raw)
  To: Baris Can Goral, allison.henderson
  Cc: davem, edumazet, horms, kuba, linux-rdma, michal.swiatkowski,
	netdev, pabeni, shankari.ak0208, skhan

在 2025/5/20 18:23, Baris Can Goral 写道:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string, resulting in potential memory
> content exposures, unbounded reads, or crashes.
> Link: https://github.com/KSPP/linux/issues/90
> 
> In addition, strscpy_pad is more appropriate because it also zero-fills
> any remaining space in the destination if the source is shorter than
> the provided buffer size.

Please don't reply to an old thread when starting a new version.

It is better to start a new thread with the new version.

Zhu Yanjun
> 
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
> ---
>   net/rds/connection.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..d62f486ab29f 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,8 +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,
> -		sizeof(cinfo->transport));
> +	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
>   	cinfo->flags = 0;
>   
>   	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
> @@ -775,8 +774,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,
> -		sizeof(cinfo6->transport));
> +	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
>   	cinfo6->flags = 0;
>   
>   	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 net-next: rds] replace strncpy with strscpy_pad
@ 2025-05-21 16:10 Baris Can Goral
  2025-05-21 17:14 ` Allison Henderson
  2025-05-26 20:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 13+ messages in thread
From: Baris Can Goral @ 2025-05-21 16:10 UTC (permalink / raw)
  To: allison.henderson, davem, edumazet, horms, kuba
  Cc: goralbaris, linux-rdma, michal.swiatkowski, netdev, pabeni,
	shankari.ak0208, skhan

The strncpy() function is actively dangerous to use since it may not
NULL-terminate the destination string, resulting in potential memory
content exposures, unbounded reads, or crashes.
Link: https://github.com/KSPP/linux/issues/90

In addition, strscpy_pad is more appropriate because it also zero-fills
any remaining space in the destination if the source is shorter than
the provided buffer size.

Signed-off-by: Baris Can Goral <goralbaris@gmail.com>
---
 net/rds/connection.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/rds/connection.c b/net/rds/connection.c
index c749c5525b40..d62f486ab29f 100644
--- a/net/rds/connection.c
+++ b/net/rds/connection.c
@@ -749,8 +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,
-		sizeof(cinfo->transport));
+	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
 	cinfo->flags = 0;
 
 	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
@@ -775,8 +774,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,
-		sizeof(cinfo6->transport));
+	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
 	cinfo6->flags = 0;
 
 	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next: rds] replace strncpy with strscpy_pad
  2025-05-21 16:10 Baris Can Goral
@ 2025-05-21 17:14 ` Allison Henderson
  2025-05-26 20:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 13+ messages in thread
From: Allison Henderson @ 2025-05-21 17:14 UTC (permalink / raw)
  To: horms@kernel.org, edumazet@google.com, davem@davemloft.net,
	goralbaris@gmail.com, kuba@kernel.org
  Cc: michal.swiatkowski@linux.intel.com, shankari.ak0208@gmail.com,
	netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	pabeni@redhat.com, skhan@linuxfoundation.org

On Wed, 2025-05-21 at 19:10 +0300, Baris Can Goral wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string, resulting in potential memory
> content exposures, unbounded reads, or crashes.
> Link: https://urldefense.com/v3/__https://github.com/KSPP/linux/issues/90__;!!ACWV5N9M2RV99hQ!PnLUmB9dUMUGdB0dDAEBM6IyoQVEtGLUn755qxSKNTBnGdMzPyDN0cDAS8qijzYFKXnMu3ApLnN0Jc3YBBfCtzh9$ 
> 
> In addition, strscpy_pad is more appropriate because it also zero-fills
> any remaining space in the destination if the source is shorter than
> the provided buffer size.
> 
> Signed-off-by: Baris Can Goral <goralbaris@gmail.com>

Thanks for the updates, I think this version looks correct now.

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> ---
>  net/rds/connection.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index c749c5525b40..d62f486ab29f 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
> @@ -749,8 +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,
> -		sizeof(cinfo->transport));
> +	strscpy_pad(cinfo->transport, conn->c_trans->t_name);
>  	cinfo->flags = 0;
>  
>  	rds_conn_info_set(cinfo->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
> @@ -775,8 +774,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,
> -		sizeof(cinfo6->transport));
> +	strscpy_pad(cinfo6->transport, conn->c_trans->t_name);
>  	cinfo6->flags = 0;
>  
>  	rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 net-next: rds] replace strncpy with strscpy_pad
  2025-05-21 16:10 Baris Can Goral
  2025-05-21 17:14 ` Allison Henderson
@ 2025-05-26 20:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-26 20:40 UTC (permalink / raw)
  To: Baris Can Goral
  Cc: allison.henderson, davem, edumazet, horms, kuba, linux-rdma,
	michal.swiatkowski, netdev, pabeni, shankari.ak0208, skhan

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 21 May 2025 19:10:37 +0300 you wrote:
> The strncpy() function is actively dangerous to use since it may not
> NULL-terminate the destination string, resulting in potential memory
> content exposures, unbounded reads, or crashes.
> Link: https://github.com/KSPP/linux/issues/90
> 
> In addition, strscpy_pad is more appropriate because it also zero-fills
> any remaining space in the destination if the source is shorter than
> the provided buffer size.
> 
> [...]

Here is the summary with links:
  - [v5,net-next:,rds] replace strncpy with strscpy_pad
    https://git.kernel.org/netdev/net-next/c/5bccdc51f90c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-05-26 20:39 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20250407183052.8763-1-goralbaris@gmail.com>
2025-04-08 18:22 ` [PATCH] net: rds transform strncpy to strscpy Simon Horman
2025-04-08 21:21   ` [PATCH net-next v2] Replace strncpy with strscpy Baris Can Goral
2025-05-17 18:12     ` goralbaris
2025-05-18  9:00       ` Simon Horman
2025-05-18 19:53         ` [PATCH v3 net-next: rds] replace strncpy with strscpy_pad goralbaris
2025-05-19  7:01           ` Michal Swiatkowski
2025-05-19 12:51             ` [PATCH v4 " Baris Can Goral
2025-05-19 21:15               ` Allison Henderson
2025-05-20 16:23                 ` [PATCH v5 " Baris Can Goral
2025-05-20 21:13                   ` Zhu Yanjun
2025-05-21 16:10 Baris Can Goral
2025-05-21 17:14 ` Allison Henderson
2025-05-26 20:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox