strlcpy() reads the entire source buffer first. This read may exceed
the destination size limit. This is both inefficient and can lead
to linear read overflows if a source string is not NUL-terminated[1].
Additionally, it returns the size of the source string, not the
resulting size of the destination string. In an effort to remove strlcpy()
completely[2], replace strlcpy() here with strscpy().

Explicitly handle the truncation case by returning the size of the
resulting string.

If "nodename" was ever longer than sizeof(clnt->cl_nodename) - 1, this
change will fix a bug where clnt->cl_nodelen would end up thinking there
were more characters in clnt->cl_nodename than there actually were,
which might have lead to kernel memory content exposures.

Cc: Trond Myklebust <trond.mykleb...@hammerspace.com>
Cc: Anna Schumaker <a...@kernel.org>
Cc: Chuck Lever <chuck.le...@oracle.com>
Cc: Jeff Layton <jlay...@kernel.org>
Cc: Neil Brown <ne...@suse.de>
Cc: Olga Kornievskaia <ko...@netapp.com>
Cc: Dai Ngo <dai....@oracle.com>
Cc: Tom Talpey <t...@talpey.com>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Eric Dumazet <eduma...@google.com>
Cc: Jakub Kicinski <k...@kernel.org>
Cc: Paolo Abeni <pab...@redhat.com>
Cc: linux-...@vger.kernel.org
Cc: net...@vger.kernel.org
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1]
Link: https://github.com/KSPP/linux/issues/89 [2]
Co-developed-by: Azeem Shaikh <azeemshaik...@gmail.com>
Signed-off-by: Azeem Shaikh <azeemshaik...@gmail.com>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 net/sunrpc/clnt.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index daa9582ec861..7afe02bdea4a 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -287,8 +287,14 @@ static struct rpc_xprt *rpc_clnt_set_transport(struct 
rpc_clnt *clnt,
 
 static void rpc_clnt_set_nodename(struct rpc_clnt *clnt, const char *nodename)
 {
-       clnt->cl_nodelen = strlcpy(clnt->cl_nodename,
-                       nodename, sizeof(clnt->cl_nodename));
+       ssize_t copied;
+
+       copied = strscpy(clnt->cl_nodename,
+                        nodename, sizeof(clnt->cl_nodename));
+
+       clnt->cl_nodelen = copied < 0
+                               ? sizeof(clnt->cl_nodename) - 1
+                               : copied;
 }
 
 static int rpc_client_register(struct rpc_clnt *clnt,
-- 
2.34.1


Reply via email to