| Message ID | 20250408194153.6570-1-pranav.tyagi03@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | net: rds: replace strncpy with memcpy | expand |
On 4/8/2025 12:41 PM, Pranav Tyagi wrote: > > Replace deprecated strncpy() function with memcpy() I suspect that strtomem() is a better answer here than a raw memcpy() - it already has all the strnlen() and min() stuff baked into it, along with some other compile-time checking. > as the destination buffer is length bounded > and not required to be NUL-terminated Are you sure that null-termination is not required? I'm not familiar with this bit of code, but the definitions of both of the .transport[] fields do say /* null term ascii */ sln > > Signed-off-by: Pranav Tyagi <pranav.tyagi03@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..3718c3edb32e 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)); > + memcpy(cinfo->transport, conn->c_trans->t_name, min(sizeof(cinfo->transport), strnlen(conn->c_trans->t_name, sizeof(cinfo->transport)))); > 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)); > + memcpy(cinfo6->transport, conn->c_trans->t_name, min(sizeof(cinfo6->transport), strnlen(conn->c_trans->t_name, sizeof(cinfo6->transport)))); > cinfo6->flags = 0; > > rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags), > -- > 2.49.0 > >
On Tue, 2025-04-08 at 14:18 -0700, Nelson, Shannon wrote: > On 4/8/2025 12:41 PM, Pranav Tyagi wrote: > > > > Replace deprecated strncpy() function with memcpy() > > I suspect that strtomem() is a better answer here than a raw memcpy() - > it already has all the strnlen() and min() stuff baked into it, along > with some other compile-time checking. > > > as the destination buffer is length bounded > > and not required to be NUL-terminated > > Are you sure that null-termination is not required? I'm not familiar > with this bit of code, but the definitions of both of the .transport[] > fields do say /* null term ascii */ > > sln > Hi all, It appears that the transport names are null-terminated. Looking at rds_ib_transport, rds_tcp_transport, and rds_loop_transport, the t_name member is initialized to "infiniband", "tcp", or "loop", respectively— which include the null terminator. Given that, I think strscpy seems to be the appropriate function to use here. However, it looks like Baris has already submitted a similar patch yesterday, and unfortunately, we can't accept both. That said, thank you very much for your contribution—we really appreciate it!
diff --git a/net/rds/connection.c b/net/rds/connection.c index c749c5525b40..3718c3edb32e 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)); + memcpy(cinfo->transport, conn->c_trans->t_name, min(sizeof(cinfo->transport), strnlen(conn->c_trans->t_name, sizeof(cinfo->transport)))); 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)); + memcpy(cinfo6->transport, conn->c_trans->t_name, min(sizeof(cinfo6->transport), strnlen(conn->c_trans->t_name, sizeof(cinfo6->transport)))); cinfo6->flags = 0; rds_conn_info_set(cinfo6->flags, test_bit(RDS_IN_XMIT, &cp->cp_flags),
Replace deprecated strncpy() function with memcpy() as the destination buffer is length bounded and not required to be NUL-terminated Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com> --- net/rds/connection.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)