linux-kernel - [PATCH 2/2] SUNRPC: Optimize 'svc_print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 25 Mar 2020 08:04:52 +0100
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     trond.myklebust@...merspace.com, anna.schumaker@...app.com,
        bfields@...ldses.org, chuck.lever@...cle.com, davem@...emloft.net,
        kuba@...nel.org, gnb@....com, neilb@...e.de,
        tom@...ngridcomputing.com
Cc:     linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org,
        Christophe JAILLET <christophe.jaillet@...adoo.fr>
Subject: [PATCH 2/2] SUNRPC: Optimize 'svc_print_xprts()'

Using 'snprintf' is safer than 'sprintf' because it can avoid a buffer
overflow.
The return value can also be used to avoid a strlen a call.

Finally, we know where we need to copy and the length to copy, so, we
can save a few cycles by rearraging the code and using a memcpy instead of
a strcat.

Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
---
This patch should have no functionnal change.
We could go further, use scnprintf and write directly in the destination
buffer. However, this could lead to a truncated last line.
---
 net/sunrpc/svc_xprt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index df39e7b8b06c..6df861650040 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -118,12 +118,12 @@ int svc_print_xprts(char *buf, int maxlen)
 	list_for_each_entry(xcl, &svc_xprt_class_list, xcl_list) {
 		int slen;
 
-		sprintf(tmpstr, "%s %d\n", xcl->xcl_name, xcl->xcl_max_payload);
-		slen = strlen(tmpstr);
-		if (len + slen >= maxlen)
+		slen = snprintf(tmpstr, sizeof(tmpstr), "%s %d\n",
+				xcl->xcl_name, xcl->xcl_max_payload);
+		if (slen >= sizeof(tmpstr) || len + slen >= maxlen)
 			break;
+		memcpy(buf + len, tmpstr, slen + 1);
 		len += slen;
-		strcat(buf, tmpstr);
 	}
 	spin_unlock(&svc_xprt_class_lock);
 
-- 
2.20.1