SUNRPC: Replace strlcpy() with strscpy()
authorKees Cook <keescook@chromium.org>
Tue, 14 Nov 2023 17:54:18 +0000 (09:54 -0800)
committerKees Cook <keescook@chromium.org>
Thu, 30 Nov 2023 20:18:36 +0000 (12:18 -0800)
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.myklebust@hammerspace.com>
Cc: Anna Schumaker <anna@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Jeff Layton <jlayton@kernel.org>
Cc: Neil Brown <neilb@suse.de>
Cc: Olga Kornievskaia <kolga@netapp.com>
Cc: Dai Ngo <Dai.Ngo@oracle.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: linux-nfs@vger.kernel.org
Cc: netdev@vger.kernel.org
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy
Link: https://github.com/KSPP/linux/issues/89
Co-developed-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Signed-off-by: Azeem Shaikh <azeemshaikh38@gmail.com>
Reviewed-by: NeilBrown <neilb@suse.de>
Link: https://lore.kernel.org/r/20231114175407.work.410-kees@kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
net/sunrpc/clnt.c

index daa9582ec861aa3a065f81a621e364961c8f4604..7afe02bdea4a678fd334b2c6cde74058eba37999 100644 (file)
@@ -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,