From 4cd5dc751a5889b5b37aa88752e33a58d8f6b624 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:31:32 -0400 Subject: NFSD: Refactor transport removal out of __write_ports() Clean up: Refactor transport removal out of __write_ports() to make it easier to understand and maintain. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 53 +++++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 24 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index af16849d243..2c1dce8e9a0 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -910,6 +910,31 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size) return rv; } +/* + * A transport listener is removed by writing a "-", it's transport + * name, and it's port number. + */ +static ssize_t __write_ports_delxprt(char *buf) +{ + struct svc_xprt *xprt; + char transport[16]; + int port; + + if (sscanf(&buf[1], "%15s %4u", transport, &port) != 2) + return -EINVAL; + + if (port < 1 || port > USHORT_MAX || nfsd_serv == NULL) + return -EINVAL; + + xprt = svc_find_xprt(nfsd_serv, transport, AF_UNSPEC, port); + if (xprt == NULL) + return -ENOTCONN; + + svc_close_xprt(xprt); + svc_xprt_put(xprt); + return 0; +} + static ssize_t __write_ports(struct file *file, char *buf, size_t size) { if (size == 0) { @@ -984,30 +1009,10 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) return err < 0 ? err : 0; } } - /* - * Remove a transport by writing it's transport name and port number - */ - if (buf[0] == '-' && isalpha(buf[1])) { - struct svc_xprt *xprt; - int err = -EINVAL; - char transport[16]; - int port; - if (sscanf(&buf[1], "%15s %4d", transport, &port) == 2) { - if (port < 1 || port > 65535) - return -EINVAL; - if (nfsd_serv) { - xprt = svc_find_xprt(nfsd_serv, transport, - AF_UNSPEC, port); - if (xprt) { - svc_close_xprt(xprt); - svc_xprt_put(xprt); - err = 0; - } else - err = -ENOTCONN; - } - return err < 0 ? err : 0; - } - } + + if (buf[0] == '-' && isalpha(buf[1])) + return __write_ports_delxprt(buf); + return -EINVAL; } -- cgit v1.2.3-70-g09d2 From 4eb68c266cb1754ffa0040e882882680ece8cf34 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:31:40 -0400 Subject: NFSD: Refactor transport addition out of __write_ports() Clean up: Refactor transport addition out of __write_ports() to make it easier to understand and maintain. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 56 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 23 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 2c1dce8e9a0..748532b93fd 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -910,6 +910,36 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size) return rv; } +/* + * A transport listener is added by writing it's transport name and + * a port number. + */ +static ssize_t __write_ports_addxprt(char *buf) +{ + char transport[16]; + int port, err; + + if (sscanf(buf, "%15s %4u", transport, &port) != 2) + return -EINVAL; + + if (port < 1 || port > USHORT_MAX) + return -EINVAL; + + err = nfsd_create_serv(); + if (err != 0) + return err; + + err = svc_create_xprt(nfsd_serv, transport, + PF_INET, port, SVC_SOCK_ANONYMOUS); + if (err < 0) { + /* Give a reasonable perror msg for bad transport string */ + if (err == -ENOENT) + err = -EPROTONOSUPPORT; + return err; + } + return 0; +} + /* * A transport listener is removed by writing a "-", it's transport * name, and it's port number. @@ -986,29 +1016,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) kfree(toclose); return len; } - /* - * Add a transport listener by writing it's transport name - */ - if (isalpha(buf[0])) { - int err; - char transport[16]; - int port; - if (sscanf(buf, "%15s %4d", transport, &port) == 2) { - if (port < 1 || port > 65535) - return -EINVAL; - err = nfsd_create_serv(); - if (!err) { - err = svc_create_xprt(nfsd_serv, - transport, PF_INET, port, - SVC_SOCK_ANONYMOUS); - if (err == -ENOENT) - /* Give a reasonable perror msg for - * bad transport string */ - err = -EPROTONOSUPPORT; - } - return err < 0 ? err : 0; - } - } + + if (isalpha(buf[0])) + return __write_ports_addxprt(buf); if (buf[0] == '-' && isalpha(buf[1])) return __write_ports_delxprt(buf); -- cgit v1.2.3-70-g09d2 From 82d565919aa4138bc481a7473491b71385af4018 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:31:48 -0400 Subject: NFSD: Refactor portlist socket closing into a helper Clean up: Refactor the socket closing logic out of __write_ports() to make it easier to understand and maintain. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 748532b93fd..fa268d1b775 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -910,6 +910,27 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size) return rv; } +/* + * A '-' followed by the 'name' of a socket means we close the socket. + */ +static ssize_t __write_ports_delfd(char *buf) +{ + char *toclose; + int len = 0; + + toclose = kstrdup(buf + 1, GFP_KERNEL); + if (toclose == NULL) + return -ENOMEM; + + if (nfsd_serv != NULL) + len = svc_sock_names(buf, nfsd_serv, toclose); + if (len >= 0) + lockd_down(); + + kfree(toclose); + return len; +} + /* * A transport listener is added by writing it's transport name and * a port number. @@ -1004,18 +1025,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) } return err < 0 ? err : 0; } - if (buf[0] == '-' && isdigit(buf[1])) { - char *toclose = kstrdup(buf+1, GFP_KERNEL); - int len = 0; - if (!toclose) - return -ENOMEM; - if (nfsd_serv) - len = svc_sock_names(buf, nfsd_serv, toclose); - if (len >= 0) - lockd_down(); - kfree(toclose); - return len; - } + + if (buf[0] == '-' && isdigit(buf[1])) + return __write_ports_delfd(buf); if (isalpha(buf[0])) return __write_ports_addxprt(buf); -- cgit v1.2.3-70-g09d2 From 0b7c2f6fc7e06867885c7b0f256ff5bd494e0653 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:31:55 -0400 Subject: NFSD: Refactor socket creation out of __write_ports() Clean up: Refactor the socket creation logic out of __write_ports() to make it easier to understand and maintain. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 64 ++++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 30 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index fa268d1b775..b6a847faa0b 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -910,6 +910,37 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size) return rv; } +/* + * A single 'fd' number was written, in which case it must be for + * a socket of a supported family/protocol, and we use it as an + * nfsd listener. + */ +static ssize_t __write_ports_addfd(char *buf) +{ + char *mesg = buf; + int fd, err; + + err = get_int(&mesg, &fd); + if (err != 0 || fd < 0) + return -EINVAL; + + err = nfsd_create_serv(); + if (err != 0) + return err; + + err = svc_addsock(nfsd_serv, fd, buf); + if (err >= 0) { + err = lockd_up(); + if (err < 0) + svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf); + + /* Decrease the count, but don't shut down the service */ + nfsd_serv->sv_nrthreads--; + } + + return err < 0 ? err : 0; +} + /* * A '-' followed by the 'name' of a socket means we close the socket. */ @@ -995,36 +1026,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) len = svc_xprt_names(nfsd_serv, buf, 0); return len; } - /* Either a single 'fd' number is written, in which - * case it must be for a socket of a supported family/protocol, - * and we use it as an nfsd socket, or - * A '-' followed by the 'name' of a socket in which case - * we close the socket. - */ - if (isdigit(buf[0])) { - char *mesg = buf; - int fd; - int err; - err = get_int(&mesg, &fd); - if (err) - return -EINVAL; - if (fd < 0) - return -EINVAL; - err = nfsd_create_serv(); - if (!err) { - err = svc_addsock(nfsd_serv, fd, buf); - if (err >= 0) { - err = lockd_up(); - if (err < 0) - svc_sock_names(buf+strlen(buf)+1, nfsd_serv, buf); - } - /* Decrease the count, but don't shutdown the - * the service - */ - nfsd_serv->sv_nrthreads--; - } - return err < 0 ? err : 0; - } + + if (isdigit(buf[0])) + return __write_ports_addfd(buf); if (buf[0] == '-' && isdigit(buf[1])) return __write_ports_delfd(buf); -- cgit v1.2.3-70-g09d2 From c71206a7b4e829601e44c49d8a0f4df0fdfe9302 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:03 -0400 Subject: NFSD: Note an additional requirement when passing TCP sockets to portlist User space must call listen(3) on SOCK_STREAM sockets passed into /proc/fs/nfsd/portlist, otherwise that listener is ignored. Document this. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index b6a847faa0b..d491fa9df9b 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1061,7 +1061,9 @@ static ssize_t __write_ports(struct file *file, char *buf, size_t size) * buf: C string containing an unsigned * integer value representing a bound * but unconnected socket that is to be - * used as an NFSD listener + * used as an NFSD listener; listen(3) + * must be called for a SOCK_STREAM + * socket, otherwise it is ignored * size: non-zero length of C string in @buf * Output: * On success: NFS service is started; -- cgit v1.2.3-70-g09d2 From 0a5372d8a13c3f670bf7c9c4a68ab04c3664fa28 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:10 -0400 Subject: NFSD: Finish refactoring __write_ports() Clean up: Refactor transport name listing out of __write_ports() to make it easier to understand and maintain. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index d491fa9df9b..caf4fdc674c 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -910,6 +910,17 @@ static ssize_t write_versions(struct file *file, char *buf, size_t size) return rv; } +/* + * Zero-length write. Return a list of NFSD's current listener + * transports. + */ +static ssize_t __write_ports_names(char *buf) +{ + if (nfsd_serv == NULL) + return 0; + return svc_xprt_names(nfsd_serv, buf, 0); +} + /* * A single 'fd' number was written, in which case it must be for * a socket of a supported family/protocol, and we use it as an @@ -1019,13 +1030,8 @@ static ssize_t __write_ports_delxprt(char *buf) static ssize_t __write_ports(struct file *file, char *buf, size_t size) { - if (size == 0) { - int len = 0; - - if (nfsd_serv) - len = svc_xprt_names(nfsd_serv, buf, 0); - return len; - } + if (size == 0) + return __write_ports_names(buf); if (isdigit(buf[0])) return __write_ports_addfd(buf); -- cgit v1.2.3-70-g09d2 From ea068bad27cefc71ab03230dbf01a8f8d98da5ba Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:18 -0400 Subject: NFSD: move lockd_up() before svc_addsock() Clean up. A couple of years ago, a series of commits, finishing with commit 5680c446, swapped the order of the lockd_up() and svc_addsock() calls in __write_ports(). At that time lockd_up() needed to know the transport protocol of the passed-in socket to start a listener on the same transport protocol. These days, lockd_up() doesn't take a protocol argument; it always starts both a UDP and TCP listener. It's now more straightforward to try the lockd_up() first, then do a lockd_down() if the svc_addsock() fails. Careful review of this code shows that the svc_sock_names() call is used only to close the just-opened socket in case lockd_up() fails. So it is no longer needed if lockd_up() is done first. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index caf4fdc674c..e051847b93f 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -939,17 +939,18 @@ static ssize_t __write_ports_addfd(char *buf) if (err != 0) return err; - err = svc_addsock(nfsd_serv, fd, buf); - if (err >= 0) { - err = lockd_up(); - if (err < 0) - svc_sock_names(buf + strlen(buf) + 1, nfsd_serv, buf); + err = lockd_up(); + if (err != 0) + goto out; - /* Decrease the count, but don't shut down the service */ - nfsd_serv->sv_nrthreads--; - } + err = svc_addsock(nfsd_serv, fd, buf); + if (err < 0) + lockd_down(); - return err < 0 ? err : 0; +out: + /* Decrease the count, but don't shut down the service */ + nfsd_serv->sv_nrthreads--; + return err; } /* -- cgit v1.2.3-70-g09d2 From 335c54bdc4d3bacdbd619ec95cd0b352435bd37f Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:25 -0400 Subject: 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 Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 2 +- include/linux/sunrpc/svc_xprt.h | 2 +- net/sunrpc/svc_xprt.c | 56 ++++++++++++++++++++++++++++------------- 3 files changed, 41 insertions(+), 19 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index e051847b93f..6a1cd908e6b 100644 --- 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 d790c52525c..2223ae0b5ed 100644 --- 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 f200393ac87..6f33d33cc06 100644 --- 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; } -- cgit v1.2.3-70-g09d2 From bfba9ab4c64f0e5c33930711e6c073c285e01fcf Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:33 -0400 Subject: SUNRPC: pass buffer size to svc_addsock() Adjust the synopsis of svc_addsock() to pass in the size of the output buffer. Add a documenting comment. This is a cosmetic change for now. A subsequent patch will make sure the buffer length is passed to one_sock_name(), where the length will actually be useful. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 2 +- include/linux/sunrpc/svcsock.h | 3 ++- net/sunrpc/svcsock.c | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 5 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 6a1cd908e6b..1f1c2159b80 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -943,7 +943,7 @@ static ssize_t __write_ports_addfd(char *buf) if (err != 0) goto out; - err = svc_addsock(nfsd_serv, fd, buf); + err = svc_addsock(nfsd_serv, fd, buf, SIMPLE_TRANSACTION_LIMIT); if (err < 0) lockd_down(); diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h index 483e10380aa..e23241c53f4 100644 --- a/include/linux/sunrpc/svcsock.h +++ b/include/linux/sunrpc/svcsock.h @@ -39,7 +39,8 @@ int svc_send(struct svc_rqst *); void svc_drop(struct svc_rqst *); void svc_sock_update_bufs(struct svc_serv *serv); int svc_sock_names(char *buf, struct svc_serv *serv, char *toclose); -int svc_addsock(struct svc_serv *serv, int fd, char *name_return); +int svc_addsock(struct svc_serv *serv, const int fd, + char *name_return, const size_t len); void svc_init_xprt_sock(void); void svc_cleanup_xprt_sock(void); diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 8b083283413..6bec1e25b54 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -1128,9 +1128,19 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv, return svsk; } -int svc_addsock(struct svc_serv *serv, - int fd, - char *name_return) +/** + * svc_addsock - add a listener socket to an RPC service + * @serv: pointer to RPC service to which to add a new listener + * @fd: file descriptor of the new listener + * @name_return: pointer to buffer to fill in with name of listener + * @len: size of the buffer + * + * Fills in socket name and returns positive length of name if successful. + * Name is terminated with '\n'. On error, returns a negative errno + * value. + */ +int svc_addsock(struct svc_serv *serv, const int fd, char *name_return, + const size_t len) { int err = 0; struct socket *so = sockfd_lookup(fd, &err); -- cgit v1.2.3-70-g09d2 From 8435d34dbbe75678c3cdad3d53b1e7996a79b3bf Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:32:40 -0400 Subject: SUNRPC: pass buffer size to svc_sock_names() Adjust the synopsis of svc_sock_names() to pass in the size of the output buffer. Add a documenting comment. This is a cosmetic change for now. A subsequent patch will make sure the buffer length is passed to one_sock_name(), where the length will actually be useful. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 3 ++- include/linux/sunrpc/svcsock.h | 4 +++- net/sunrpc/svcsock.c | 19 +++++++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 1f1c2159b80..b64a7fbfccf 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -966,7 +966,8 @@ static ssize_t __write_ports_delfd(char *buf) return -ENOMEM; if (nfsd_serv != NULL) - len = svc_sock_names(buf, nfsd_serv, toclose); + len = svc_sock_names(nfsd_serv, buf, + SIMPLE_TRANSACTION_LIMIT, toclose); if (len >= 0) lockd_down(); diff --git a/include/linux/sunrpc/svcsock.h b/include/linux/sunrpc/svcsock.h index e23241c53f4..82716313894 100644 --- a/include/linux/sunrpc/svcsock.h +++ b/include/linux/sunrpc/svcsock.h @@ -38,7 +38,9 @@ int svc_recv(struct svc_rqst *, long); int svc_send(struct svc_rqst *); void svc_drop(struct svc_rqst *); void svc_sock_update_bufs(struct svc_serv *serv); -int svc_sock_names(char *buf, struct svc_serv *serv, char *toclose); +int svc_sock_names(struct svc_serv *serv, char *buf, + const size_t buflen, + const char *toclose); int svc_addsock(struct svc_serv *serv, const int fd, char *name_return, const size_t len); void svc_init_xprt_sock(void); diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 6bec1e25b54..032b52ea954 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -259,8 +259,23 @@ static int one_sock_name(char *buf, struct svc_sock *svsk) return len; } -int -svc_sock_names(char *buf, struct svc_serv *serv, char *toclose) +/** + * svc_sock_names - construct a list of listener names in a string + * @serv: pointer to RPC service + * @buf: pointer to a buffer to fill in with socket names + * @buflen: size of the buffer to be filled + * @toclose: pointer to '\0'-terminated C string containing the name + * of a listener to be closed + * + * Fills in @buf with a '\n'-separated list of names of listener + * sockets. If @toclose is not NULL, the socket named by @toclose + * is closed, and is not included in the output list. + * + * Returns positive length of the socket name string, or a negative + * errno value on error. + */ +int svc_sock_names(struct svc_serv *serv, char *buf, const size_t buflen, + const char *toclose) { struct svc_sock *svsk, *closesk = NULL; int len = 0; -- cgit v1.2.3-70-g09d2 From 3d72ab8fdd44c872633b210dd1a4afd2910d0bbb Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:33:10 -0400 Subject: NFSD: Stricter buffer size checking in write_recoverydir() While it's not likely a pathname will be longer than SIMPLE_TRANSACTION_SIZE, we should be more careful about just plopping it into the output buffer without bounds checking. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index b64a7fbfccf..c4843467cfd 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -1260,8 +1260,9 @@ static ssize_t __write_recoverydir(struct file *file, char *buf, size_t size) status = nfs4_reset_recoverydir(recdir); } - sprintf(buf, "%s\n", nfs4_recoverydir()); - return strlen(buf); + + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%s\n", + nfs4_recoverydir()); } /** -- cgit v1.2.3-70-g09d2 From 261758b5c3dfeac73ca364c47ed538f5ce4250ee Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:33:18 -0400 Subject: NFSD: Stricter buffer size checking in write_versions() While it's not likely today that there are enough NFS versions to overflow the output buffer in write_versions(), we should be more careful about detecting the end of the buffer. The number of NFS versions will only increase as NFSv4 minor versions are added. Note that this API doesn't behave the same as portlist. Here we attempt to display as many versions as will fit in the buffer, and do not provide any indication that an overflow would have occurred. I don't have any good rationale for that. Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index c4843467cfd..a152694e016 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -793,7 +793,7 @@ static ssize_t __write_versions(struct file *file, char *buf, size_t size) { char *mesg = buf; char *vers, *minorp, sign; - int len, num; + int len, num, remaining; unsigned minor; ssize_t tlen = 0; char *sep; @@ -840,32 +840,50 @@ static ssize_t __write_versions(struct file *file, char *buf, size_t size) } next: vers += len + 1; - tlen += len; } while ((len = qword_get(&mesg, vers, size)) > 0); /* If all get turned off, turn them back on, as * having no versions is BAD */ nfsd_reset_versions(); } + /* Now write current state into reply buffer */ len = 0; sep = ""; + remaining = SIMPLE_TRANSACTION_LIMIT; for (num=2 ; num <= 4 ; num++) if (nfsd_vers(num, NFSD_AVAIL)) { - len += sprintf(buf+len, "%s%c%d", sep, + len = snprintf(buf, remaining, "%s%c%d", sep, nfsd_vers(num, NFSD_TEST)?'+':'-', num); sep = " "; + + if (len > remaining) + break; + remaining -= len; + buf += len; + tlen += len; } if (nfsd_vers(4, NFSD_AVAIL)) - for (minor = 1; minor <= NFSD_SUPPORTED_MINOR_VERSION; minor++) - len += sprintf(buf+len, " %c4.%u", + for (minor = 1; minor <= NFSD_SUPPORTED_MINOR_VERSION; + minor++) { + len = snprintf(buf, remaining, " %c4.%u", (nfsd_vers(4, NFSD_TEST) && nfsd_minorversion(minor, NFSD_TEST)) ? '+' : '-', minor); - len += sprintf(buf+len, "\n"); - return len; + + if (len > remaining) + break; + remaining -= len; + buf += len; + tlen += len; + } + + len = snprintf(buf, remaining, "\n"); + if (len > remaining) + return -EINVAL; + return tlen + len; } /** -- cgit v1.2.3-70-g09d2 From e06b64050ebea1220bca3c291d0431f5206b5c95 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 23 Apr 2009 19:33:25 -0400 Subject: NFSD: Stricter buffer size checking in fs/nfsd/nfsctl.c Clean up: For consistency, handle output buffer size checking in a other nfsctl functions the same way it's done for write_versions(). Signed-off-by: Chuck Lever Signed-off-by: J. Bruce Fields --- fs/nfsd/nfsctl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index a152694e016..877e713a0fd 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -695,8 +695,9 @@ static ssize_t write_threads(struct file *file, char *buf, size_t size) if (rv) return rv; } - sprintf(buf, "%d\n", nfsd_nrthreads()); - return strlen(buf); + + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n", + nfsd_nrthreads()); } /** @@ -1197,7 +1198,9 @@ static ssize_t write_maxblksize(struct file *file, char *buf, size_t size) nfsd_max_blksize = bsize; mutex_unlock(&nfsd_mutex); } - return sprintf(buf, "%d\n", nfsd_max_blksize); + + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n", + nfsd_max_blksize); } #ifdef CONFIG_NFSD_V4 @@ -1221,8 +1224,9 @@ static ssize_t __write_leasetime(struct file *file, char *buf, size_t size) return -EINVAL; nfs4_reset_lease(lease); } - sprintf(buf, "%ld\n", nfs4_lease_time()); - return strlen(buf); + + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%ld\n", + nfs4_lease_time()); } /** -- cgit v1.2.3-70-g09d2 From 82e12fe9244ff653f703722a8937b595e10e71f4 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 16 Jun 2009 11:03:07 +1000 Subject: nfsd: don't take nfsd_mutex twice when setting number of threads. Currently when we write a number to 'threads' in nfsdfs, we take the nfsd_mutex, update the number of threads, then take the mutex again to read the number of threads. Mostly this isn't a big deal. However if we are write '0', and portmap happens to be dead, then we can get unpredictable behaviour. If the nfsd threads all got killed quickly and the last thread is waiting for portmap to respond, then the second time we take the mutex we will block waiting for the last thread. However if the nfsd threads didn't die quite that fast, then there will be no contention when we try to take the mutex again. Unpredictability isn't fun, and waiting for the last thread to exit is pointless, so avoid taking the lock twice. To achieve this, get nfsd_svc return a non-negative number of active threads when not returning a negative error. Signed-off-by: NeilBrown --- fs/nfsd/nfsctl.c | 14 +++++++++----- fs/nfsd/nfssvc.c | 6 ++++++ 2 files changed, 15 insertions(+), 5 deletions(-) (limited to 'fs/nfsd/nfsctl.c') diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c index 877e713a0fd..1250fb978ac 100644 --- a/fs/nfsd/nfsctl.c +++ b/fs/nfsd/nfsctl.c @@ -207,10 +207,14 @@ static struct file_operations pool_stats_operations = { static ssize_t write_svc(struct file *file, char *buf, size_t size) { struct nfsctl_svc *data; + int err; if (size < sizeof(*data)) return -EINVAL; data = (struct nfsctl_svc*) buf; - return nfsd_svc(data->svc_port, data->svc_nthreads); + err = nfsd_svc(data->svc_port, data->svc_nthreads); + if (err < 0) + return err; + return 0; } /** @@ -692,12 +696,12 @@ static ssize_t write_threads(struct file *file, char *buf, size_t size) if (newthreads < 0) return -EINVAL; rv = nfsd_svc(NFS_PORT, newthreads); - if (rv) + if (rv < 0) return rv; - } + } else + rv = nfsd_nrthreads(); - return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n", - nfsd_nrthreads()); + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n", rv); } /** diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c index cbba4a93578..209eaa0885d 100644 --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -413,6 +413,12 @@ nfsd_svc(unsigned short port, int nrservs) goto failure; error = svc_set_num_threads(nfsd_serv, NULL, nrservs); + if (error == 0) + /* We are holding a reference to nfsd_serv which + * we don't want to count in the return value, + * so subtract 1 + */ + error = nfsd_serv->sv_nrthreads - 1; failure: svc_destroy(nfsd_serv); /* Release server */ out: -- cgit v1.2.3-70-g09d2