* [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy [not found] <20250501012555.92688-1-rubenru09.ref@aol.com> @ 2025-05-01 1:23 ` Ruben Wauters 2025-05-01 15:39 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Ruben Wauters @ 2025-05-01 1:23 UTC (permalink / raw) To: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman Cc: Ruben Wauters, netdev, linux-kernel Use of strcpy is decpreated, replaces the use of strcpy with strscpy as recommended. I am aware there is an explicit bounds check above, however using strscpy protects against buffer overflows in any future code, and there is no good reason I can see to not use it. Signed-off-by: Ruben Wauters <rubenru09@aol.com> --- net/ipv4/ip_tunnel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 3913ec89ad20..9724bbbd0e0a 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -247,7 +247,7 @@ static struct net_device *__ip_tunnel_create(struct net *net, } else { if (strlen(ops->kind) > (IFNAMSIZ - 3)) goto failed; - strcpy(name, ops->kind); + strscpy(name, ops->kind); strcat(name, "%d"); } -- 2.48.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy 2025-05-01 1:23 ` [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy Ruben Wauters @ 2025-05-01 15:39 ` Simon Horman 2025-05-01 16:51 ` Ruben Wauters 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2025-05-01 15:39 UTC (permalink / raw) To: Ruben Wauters Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Thu, May 01, 2025 at 02:23:00AM +0100, Ruben Wauters wrote: > Use of strcpy is decpreated, replaces the use of strcpy with strscpy as > recommended. > > I am aware there is an explicit bounds check above, however using > strscpy protects against buffer overflows in any future code, and there > is no good reason I can see to not use it. Thanks, I agree. This patch doesn't buy us safety. But it doesn't lose us anything. And allows the code to move towards best practice. One thing I notices is that this change is is inconsistent with the call to the 3-argument variant of strscpy a few lines above - it should also be hte 2-argument version. Maybe that could be changed too. Maybe in a separate patch. It is customary when making such changes to add a note that strscpy() was chosen because the code expects a NUL-terminated string without zero-padding. (Which is the case due to the call to strcat().) Perhaps you could add some text to the commit message of v2 of this patch? > Signed-off-by: Ruben Wauters <rubenru09@aol.com> Reviewed-by: Simon Horman <horms@kernel.org> > --- > net/ipv4/ip_tunnel.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c > index 3913ec89ad20..9724bbbd0e0a 100644 > --- a/net/ipv4/ip_tunnel.c > +++ b/net/ipv4/ip_tunnel.c > @@ -247,7 +247,7 @@ static struct net_device *__ip_tunnel_create(struct net *net, > } else { > if (strlen(ops->kind) > (IFNAMSIZ - 3)) > goto failed; > - strcpy(name, ops->kind); > + strscpy(name, ops->kind); > strcat(name, "%d"); > } > > -- > 2.48.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy 2025-05-01 15:39 ` Simon Horman @ 2025-05-01 16:51 ` Ruben Wauters 2025-05-02 9:38 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Ruben Wauters @ 2025-05-01 16:51 UTC (permalink / raw) To: Simon Horman Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Thu, 2025-05-01 at 16:39 +0100, Simon Horman wrote: > On Thu, May 01, 2025 at 02:23:00AM +0100, Ruben Wauters wrote: > > Use of strcpy is decpreated, replaces the use of strcpy with > > strscpy as > > recommended. > > > > I am aware there is an explicit bounds check above, however using > > strscpy protects against buffer overflows in any future code, and > > there > > is no good reason I can see to not use it. > > Thanks, I agree. This patch doesn't buy us safety. But it doesn't > lose > us anything. And allows the code to move towards best practice. > > One thing I notices is that this change is is inconsistent with the > call to > the 3-argument variant of strscpy a few lines above - it should also > be hte > 2-argument version. Maybe that could be changed too. Maybe in a > separate patch. I can remove the size parameter from the above strscpy to make it consistent in v2. > It is customary when making such changes to add a note that > strscpy() was chosen because the code expects a NUL-terminated string > without zero-padding. (Which is the case due to the call to > strcat().) > Perhaps you could add some text to the commit message of v2 of this > patch? Apologies, I wasn't aware of this, I can add the text to v2. Just a point of clarification I wanted to ask, for v2 of the patch, should I include the Reviewed-by tag below? or should I remove it as there has been changes? > > Signed-off-by: Ruben Wauters <rubenru09@aol.com> > > Reviewed-by: Simon Horman <horms@kernel.org> > > > --- > > net/ipv4/ip_tunnel.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c > > index 3913ec89ad20..9724bbbd0e0a 100644 > > --- a/net/ipv4/ip_tunnel.c > > +++ b/net/ipv4/ip_tunnel.c > > @@ -247,7 +247,7 @@ static struct net_device > > *__ip_tunnel_create(struct net *net, > > } else { > > if (strlen(ops->kind) > (IFNAMSIZ - 3)) > > goto failed; > > - strcpy(name, ops->kind); > > + strscpy(name, ops->kind); > > strcat(name, "%d"); > > } > > > > -- > > 2.48.1 > > > > Ruben Wauters ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy 2025-05-01 16:51 ` Ruben Wauters @ 2025-05-02 9:38 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2025-05-02 9:38 UTC (permalink / raw) To: Ruben Wauters Cc: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Thu, May 01, 2025 at 05:51:08PM +0100, Ruben Wauters wrote: > On Thu, 2025-05-01 at 16:39 +0100, Simon Horman wrote: > > On Thu, May 01, 2025 at 02:23:00AM +0100, Ruben Wauters wrote: > > > Use of strcpy is decpreated, replaces the use of strcpy with > > > strscpy as > > > recommended. > > > > > > I am aware there is an explicit bounds check above, however using > > > strscpy protects against buffer overflows in any future code, and > > > there > > > is no good reason I can see to not use it. > > > > Thanks, I agree. This patch doesn't buy us safety. But it doesn't > > lose > > us anything. And allows the code to move towards best practice. > > > > One thing I notices is that this change is is inconsistent with the > > call to > > the 3-argument variant of strscpy a few lines above - it should also > > be hte > > 2-argument version. Maybe that could be changed too. Maybe in a > > separate patch. > > > I can remove the size parameter from the above strscpy to make it > consistent in v2. > > > It is customary when making such changes to add a note that > > strscpy() was chosen because the code expects a NUL-terminated string > > without zero-padding. (Which is the case due to the call to > > strcat().) > > Perhaps you could add some text to the commit message of v2 of this > > patch? > > Apologies, I wasn't aware of this, I can add the text to v2. > > Just a point of clarification I wanted to ask, for v2 of the patch, > should I include the Reviewed-by tag below? or should I remove it as > there has been changes? I think you can include it unless the changes turn out to be materially different to what has been discussed in this thread. But if in doubt, drop it. > > > > > Signed-off-by: Ruben Wauters <rubenru09@aol.com> > > > > Reviewed-by: Simon Horman <horms@kernel.org> ... ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-05-02 9:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250501012555.92688-1-rubenru09.ref@aol.com>
2025-05-01 1:23 ` [PATCH net-next] ipv4: ip_tunnel: Replace strcpy use with strscpy Ruben Wauters
2025-05-01 15:39 ` Simon Horman
2025-05-01 16:51 ` Ruben Wauters
2025-05-02 9:38 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).