Hi Christophe,I guess, that it us unlikely and that the 80 chars buffer is big enough.
On Mar 25, 2020, at 3:04 AM, Christophe JAILLET <christophe.jaillet@xxxxxxxxxx> wrote:That's true as a general statement, but how likely is such an
Using 'snprintf' is safer than 'sprintf' because it can avoid a buffer
overflow.
overflow to occur here?
The return value can also be used to avoid a strlen a call.That's also true of sprintf, isn't it?
Finally, we know where we need to copy and the length to copy, so, weI would be OK with squashing these two patches together. I don't
can save a few cycles by rearraging the code and using a memcpy instead of
a strcat.
see the need to keep the two changes separated.
Signed-off-by: Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>That's exactly what this function is trying to avoid. As part of any
---
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.
change in this area, it would be good to replace the current block
comment before this function with a Doxygen-format comment that
documents that goal.
---IMO replacing the strcat makes the code harder to read, and this
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);
is certainly not a performance path. Can you drop that part of the
patch?
}--
spin_unlock(&svc_xprt_class_lock);
--
2.20.1
Chuck Lever