NFSD: Prevent a buffer overflow in svc_xprt_names()

The svc_xprt_names() function can overflow its buffer if it's so near
the end of the passed in buffer that the "name too long" string still
doesn't fit.  Of course, it could never tell if it was near the end
of the passed in buffer, since its only caller passes in zero as the
buffer length.

Let's make this API a little safer.

Change svc_xprt_names() so it *always* checks for a buffer overflow,
and change its only caller to pass in the correct buffer length.

If svc_xprt_names() does overflow its buffer, it now fails with an
ENAMETOOLONG errno, instead of trying to write a message at the end
of the buffer.  I don't like this much, but I can't figure out a clean
way that's always safe to return some of the names, *and* an
indication that the buffer was not long enough.

The displayed error when doing a 'cat /proc/fs/nfsd/portlist' is
"File name too long".

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index e051847..6a1cd90 100644 (file)
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -918,7 +918,7 @@ static ssize_t __write_ports_names(char *buf)
 {
        if (nfsd_serv == NULL)
                return 0;
-       return svc_xprt_names(nfsd_serv, buf, 0);
+       return svc_xprt_names(nfsd_serv, buf, SIMPLE_TRANSACTION_LIMIT);
 }
 
 /*

diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h
index d790c52..2223ae0 100644 (file)
--- a/include/linux/sunrpc/svc_xprt.h
+++ b/include/linux/sunrpc/svc_xprt.h
@@ -83,7 +83,7 @@ int   svc_port_is_privileged(struct sockaddr *sin);
 int    svc_print_xprts(char *buf, int maxlen);
 struct svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
                        const sa_family_t af, const unsigned short port);
-int    svc_xprt_names(struct svc_serv *serv, char *buf, int buflen);
+int    svc_xprt_names(struct svc_serv *serv, char *buf, const int buflen);
 
 static inline void svc_xprt_get(struct svc_xprt *xprt)
 {

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index f200393..6f33d33 100644 (file)
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -1098,36 +1098,58 @@ struct svc_xprt *svc_find_xprt(struct svc_serv *serv, const char *xcl_name,
 }
 EXPORT_SYMBOL_GPL(svc_find_xprt);
 
-/*
- * Format a buffer with a list of the active transports. A zero for
- * the buflen parameter disables target buffer overflow checking.
+static int svc_one_xprt_name(const struct svc_xprt *xprt,
+                            char *pos, int remaining)
+{
+       int len;
+
+       len = snprintf(pos, remaining, "%s %u\n",
+                       xprt->xpt_class->xcl_name,
+                       svc_xprt_local_port(xprt));
+       if (len >= remaining)
+               return -ENAMETOOLONG;
+       return len;
+}
+
+/**
+ * svc_xprt_names - format a buffer with a list of transport names
+ * @serv: pointer to an RPC service
+ * @buf: pointer to a buffer to be filled in
+ * @buflen: length of buffer to be filled in
+ *
+ * Fills in @buf with a string containing a list of transport names,
+ * each name terminated with '\n'.
+ *
+ * Returns positive length of the filled-in string on success; otherwise
+ * a negative errno value is returned if an error occurs.
  */
-int svc_xprt_names(struct svc_serv *serv, char *buf, int buflen)
+int svc_xprt_names(struct svc_serv *serv, char *buf, const int buflen)
 {
        struct svc_xprt *xprt;
-       char xprt_str[64];
-       int totlen = 0;
-       int len;
+       int len, totlen;
+       char *pos;
 
        /* Sanity check args */
        if (!serv)
                return 0;
 
        spin_lock_bh(&serv->sv_lock);
+
+       pos = buf;
+       totlen = 0;
        list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
-               len = snprintf(xprt_str, sizeof(xprt_str),
-                              "%s %d\n", xprt->xpt_class->xcl_name,
-                              svc_xprt_local_port(xprt));
-               /* If the string was truncated, replace with error string */
-               if (len >= sizeof(xprt_str))
-                       strcpy(xprt_str, "name-too-long\n");
-               /* Don't overflow buffer */
-               len = strlen(xprt_str);
-               if (buflen && (len + totlen >= buflen))
+               len = svc_one_xprt_name(xprt, pos, buflen - totlen);
+               if (len < 0) {
+                       *buf = '\0';
+                       totlen = len;
+               }
+               if (len <= 0)
                        break;
-               strcpy(buf+totlen, xprt_str);
+
+               pos += len;
                totlen += len;
        }
+
        spin_unlock_bh(&serv->sv_lock);
        return totlen;
 }