| Message ID | 20240714041111.it.918-kees@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a3bfc095060b51f0198755020daf80d1e14413b1 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy() | expand |
On Sat, Jul 13, 2024 at 09:11:15PM -0700, Kees Cook wrote: > Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() > and tcp_get_default_congestion_control(). The callers use the results as > standard C strings (via nla_put_string() and proc handlers respectively), > so trailing padding is not needed. > > Since passing the destination buffer arguments decays it to a pointer, > the size can't be trivially determined by the compiler. ca->name is > the same length in both cases, so strscpy() won't fail (when ca->name > is NUL-terminated). Include the length explicitly instead of using the > 2-argument strscpy(). > > Link: https://github.com/KSPP/linux/issues/90 [1] > Signed-off-by: Kees Cook <kees@kernel.org> nit: Looking at git history, the subject prefix should probably be 'tcp'. And it would be best to explicitly target the patch against net-next. Subject: [PATCH net-next v2] tcp: ... That notwithstanding, this looks good to me. Reviewed-by: Simon Horman <horms@kernel.org> ...
On 7/15/24 11:41, Simon Horman wrote: > On Sat, Jul 13, 2024 at 09:11:15PM -0700, Kees Cook wrote: >> Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() >> and tcp_get_default_congestion_control(). The callers use the results as >> standard C strings (via nla_put_string() and proc handlers respectively), >> so trailing padding is not needed. >> >> Since passing the destination buffer arguments decays it to a pointer, >> the size can't be trivially determined by the compiler. ca->name is >> the same length in both cases, so strscpy() won't fail (when ca->name >> is NUL-terminated). Include the length explicitly instead of using the >> 2-argument strscpy(). >> >> Link: https://github.com/KSPP/linux/issues/90 [1] >> Signed-off-by: Kees Cook <kees@kernel.org> > > nit: Looking at git history, the subject prefix should probably be 'tcp'. > And it would be best to explicitly target the patch against net-next. > > Subject: [PATCH net-next v2] tcp: ... > > That notwithstanding, this looks good to me. > > Reviewed-by: Simon Horman <horms@kernel.org> @Eric: I can fix the prefix when applying the patch. Please LMK if you prefer otherwise. Thanks, Paolo
On Tue, Jul 16, 2024 at 4:32 AM Paolo Abeni <pabeni@redhat.com> wrote: > > On 7/15/24 11:41, Simon Horman wrote: > > On Sat, Jul 13, 2024 at 09:11:15PM -0700, Kees Cook wrote: > >> Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() > >> and tcp_get_default_congestion_control(). The callers use the results as > >> standard C strings (via nla_put_string() and proc handlers respectively), > >> so trailing padding is not needed. > >> > >> Since passing the destination buffer arguments decays it to a pointer, > >> the size can't be trivially determined by the compiler. ca->name is > >> the same length in both cases, so strscpy() won't fail (when ca->name > >> is NUL-terminated). Include the length explicitly instead of using the > >> 2-argument strscpy(). > >> > >> Link: https://github.com/KSPP/linux/issues/90 [1] > >> Signed-off-by: Kees Cook <kees@kernel.org> > > > > nit: Looking at git history, the subject prefix should probably be 'tcp'. > > And it would be best to explicitly target the patch against net-next. > > > > Subject: [PATCH net-next v2] tcp: ... > > > > That notwithstanding, this looks good to me. > > > > Reviewed-by: Simon Horman <horms@kernel.org> > > @Eric: I can fix the prefix when applying the patch. Please LMK if you > prefer otherwise. Sure thing, thanks for taking care of this Paolo, Simon, and Kees. Reviewed-by: Eric Dumazet <edumazet@google.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sat, 13 Jul 2024 21:11:15 -0700 you wrote: > Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() > and tcp_get_default_congestion_control(). The callers use the results as > standard C strings (via nla_put_string() and proc handlers respectively), > so trailing padding is not needed. > > Since passing the destination buffer arguments decays it to a pointer, > the size can't be trivially determined by the compiler. ca->name is > the same length in both cases, so strscpy() won't fail (when ca->name > is NUL-terminated). Include the length explicitly instead of using the > 2-argument strscpy(). > > [...] Here is the summary with links: - [v2] net/ipv4/tcp_cong: Replace strncpy() with strscpy() https://git.kernel.org/netdev/net-next/c/a3bfc095060b You are awesome, thank you!
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c index 28ffcfbeef14..874531c7c08b 100644 --- a/net/ipv4/tcp_cong.c +++ b/net/ipv4/tcp_cong.c @@ -203,9 +203,10 @@ char *tcp_ca_get_name_by_key(u32 key, char *buffer) rcu_read_lock(); ca = tcp_ca_find_key(key); - if (ca) - ret = strncpy(buffer, ca->name, - TCP_CA_NAME_MAX); + if (ca) { + strscpy(buffer, ca->name, TCP_CA_NAME_MAX); + ret = buffer; + } rcu_read_unlock(); return ret; @@ -338,7 +339,7 @@ void tcp_get_default_congestion_control(struct net *net, char *name) rcu_read_lock(); ca = rcu_dereference(net->ipv4.tcp_congestion_control); - strncpy(name, ca->name, TCP_CA_NAME_MAX); + strscpy(name, ca->name, TCP_CA_NAME_MAX); rcu_read_unlock(); }
Replace the deprecated[1] uses of strncpy() in tcp_ca_get_name_by_key() and tcp_get_default_congestion_control(). The callers use the results as standard C strings (via nla_put_string() and proc handlers respectively), so trailing padding is not needed. Since passing the destination buffer arguments decays it to a pointer, the size can't be trivially determined by the compiler. ca->name is the same length in both cases, so strscpy() won't fail (when ca->name is NUL-terminated). Include the length explicitly instead of using the 2-argument strscpy(). Link: https://github.com/KSPP/linux/issues/90 [1] Signed-off-by: Kees Cook <kees@kernel.org> --- v2: add tcp_get_default_congestion_control() conversion v1: https://lore.kernel.org/lkml/20240711171652.work.887-kees@kernel.org/ Cc: Eric Dumazet <edumazet@google.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: David Ahern <dsahern@kernel.org> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: netdev@vger.kernel.org --- net/ipv4/tcp_cong.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)