[PATCH RFC nfs-utils v2 00/10] nfsdctl: allow it to handle rpcbind registration and deregistration

[PATCH RFC nfs-utils v2 00/10] nfsdctl: allow it to handle rpcbind registration and deregistration

[Jeff Layton]

This version fixes some problems that Sashiko flagged in review.

This patchset adds support for NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND,
which tells the kernel to not do rpcbind registration and to instead
allow userland to handle it. After setting listeners, nfsdctl will then
call out to rpcbind using the normal libtirpc client interfaces.

This is a companion set to the kernel set I recently posted (and which
also should have been marked RFC).

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v2:
- Fixes for bugs v1 introduced are folded into the patches that
  introduced them. Three pre-existing bugs found along the way are
  separate patches at the end, so the set is 10 patches instead of 7.
- "register nfsd listeners with rpcbind": register only once the
  listener_set reply has been parsed. On a reply carrying no rpcbind
  attributes the sweep cleared every NFS and NFSACL entry and put
  nothing back, leaving a running server advertising nothing. Also
  abort rather than send a listener_set that lost the ownership flag.
- "clear rpcbind entries when the serv is torn down" (was "... when the
  last thread stops"): also sweep on the autostart teardown path.
- New, pre-existing: fix a heap overflow in autostart_func(). A failed
  listener set zeroed the pool count before the thread array was sized,
  but the array is filled from the nfs.conf list, so any [nfsd] threads
  setting wrote past the allocation.
- New, pre-existing: reject a listener insertion when the table is full,
  instead of writing one element past nfsd_sockets[].
- New, pre-existing: check the nla_put() calls in set_listeners().
- Link to v1: https://lore.kernel.org/r/20260910-nfsdctl-rpcb-v1-0-4bba1e7a3a43@kernel.org

---
Jeff Layton (10):
      nfsdctl: sync nfsd_netlink.h with the kernel
      nfsdctl: probe the policy of any command
      nfsdctl: share the listener nest parser
      nfsdctl: add rpcbind registration helpers
      nfsdctl: register nfsd listeners with rpcbind
      nfsdctl: clear rpcbind entries when the serv is torn down
      nfsdctl: add missing break in update_listeners()
      nfsdctl: fix heap overflow in autostart_func() listener cleanup
      nfsdctl: reject a listener insertion when the table is full
      nfsdctl: check the netlink attribute puts in set_listeners()

 configure.ac                   |   2 +-
 support/include/nfsd_netlink.h |  18 +++
 utils/nfsdctl/Makefile.am      |  10 +-
 utils/nfsdctl/nfsdctl.8        |  16 +++
 utils/nfsdctl/nfsdctl.adoc     |  17 +++
 utils/nfsdctl/nfsdctl.c        | 256 +++++++++++++++++++++++++++++++++--------
 utils/nfsdctl/rpcbind.c        | 251 ++++++++++++++++++++++++++++++++++++++++
 utils/nfsdctl/rpcbind.h        |  42 +++++++
 8 files changed, 562 insertions(+), 50 deletions(-)
---
base-commit: 630393e9c73667b3fda6a7507cb5171ec0ec38dc
change-id: 20260908-nfsdctl-rpcb-adc5a1fc090b

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH RFC nfs-utils v2 01/10] nfsdctl: sync nfsd_netlink.h with the kernel

[Jeff Layton]

Import the attributes that let a caller own rpcbind registration:

- NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND on the request
- NFSD_A_SERVER_SOCK_RPCBIND, and the NFSD_A_RPCBIND_* nest, on the reply

The file comes from include/uapi/linux/nfsd_netlink.h. No code reads the
new attributes yet.

Bump the USE_SYSTEM_NFSD_NETLINK_H probe to one of the new attributes.
It tested NFSD_CMD_SERVER_STATS_GET, which has been in the uapi header
since v6.9, so any host with headers that new picks the system copy and
loses everything imported here.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 configure.ac                   |  2 +-
 support/include/nfsd_netlink.h | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 2ce1e129e5d4..d9d268975d7d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -259,7 +259,7 @@ AC_CHECK_HEADERS(linux/nfsd_netlink.h)
 
 # ensure the system netlink headers have the latest features
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/nfsd_netlink.h>]],
-			                   [[int foo = NFSD_CMD_SERVER_STATS_GET;]])],
+			                   [[int foo = NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND;]])],
 			   [AC_DEFINE([USE_SYSTEM_NFSD_NETLINK_H], 1,
 				      ["Use system's linux/nfsd_netlink.h"])])
 AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <linux/lockd_netlink.h>]],
diff --git a/support/include/nfsd_netlink.h b/support/include/nfsd_netlink.h
index 87da1d0bb21e..400322857d16 100644
--- a/support/include/nfsd_netlink.h
+++ b/support/include/nfsd_netlink.h
@@ -50,6 +50,13 @@ enum nfsd_xprtsec_mode {
 	NFSD_XPRTSEC_MODE_MTLS = 4,
 };
 
+/*
+ * Constraints that apply to an rpcbind registration.
+ */
+enum nfsd_rpcbind_flags {
+	NFSD_RPCBIND_FLAGS_NO_UDP = 1,
+};
+
 enum {
 	NFSD_A_CACHE_NOTIFY_CACHE_TYPE = 1,
 
@@ -113,8 +120,19 @@ enum {
 	NFSD_A_SOCK_MAX = (__NFSD_A_SOCK_MAX - 1)
 };
 
+enum {
+	NFSD_A_RPCBIND_PROGRAM = 1,
+	NFSD_A_RPCBIND_VERSION,
+	NFSD_A_RPCBIND_FLAGS,
+
+	__NFSD_A_RPCBIND_MAX,
+	NFSD_A_RPCBIND_MAX = (__NFSD_A_RPCBIND_MAX - 1)
+};
+
 enum {
 	NFSD_A_SERVER_SOCK_ADDR = 1,
+	NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND,
+	NFSD_A_SERVER_SOCK_RPCBIND,
 
 	__NFSD_A_SERVER_SOCK_MAX,
 	NFSD_A_SERVER_SOCK_MAX = (__NFSD_A_SERVER_SOCK_MAX - 1)

-- 
2.55.0

[PATCH RFC nfs-utils v2 02/10] nfsdctl: probe the policy of any command

[Jeff Layton]

query_nfsd_nl_policy() asks CTRL_CMD_GETPOLICY about NFSD_CMD_THREADS_SET
only. getpolicy_handler() writes the highest attribute straight into a
global. A second command cannot use either function.

query_nfsd_nl_cmd_policy() now takes the command number.
getpolicy_handler() now writes through its callback argument.
query_nfsd_nl_policy() keeps its name and still probes THREADS_SET.

No functional change.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index c7126748dac9..8733f0377208 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -500,6 +500,7 @@ static int nfsd_nl_family_setup(struct nl_sock *sock)
 static int getpolicy_handler(struct nl_msg *msg, void *arg)
 {
 	struct genlmsghdr *gnlh = nlmsg_data(nlmsg_hdr(msg));
+	int *max_nlattr = arg;
 	struct nlattr *attr;
 	int rem;
 
@@ -512,8 +513,8 @@ static int getpolicy_handler(struct nl_msg *msg, void *arg)
 				nla_for_each_nested(b, a, j) {
 					int idx = nla_type(b);
 
-					if (nfsd_threads_max_nlattr < idx)
-						nfsd_threads_max_nlattr = idx;
+					if (*max_nlattr < idx)
+						*max_nlattr = idx;
 				}
 			}
 		}
@@ -521,7 +522,8 @@ static int getpolicy_handler(struct nl_msg *msg, void *arg)
 	return NL_SKIP;
 }
 
-static int query_nfsd_nl_policy(struct nl_sock *sock)
+static int query_nfsd_nl_cmd_policy(struct nl_sock *sock, int cmd,
+				    int *max_nlattr)
 {
 	struct genlmsghdr *ghdr;
 	struct nlmsghdr *nlh;
@@ -549,7 +551,7 @@ static int query_nfsd_nl_policy(struct nl_sock *sock)
 	}
 
 	nla_put_u16(msg, CTRL_ATTR_FAMILY_ID, nfsd_nl_family);
-	nla_put_u32(msg, CTRL_ATTR_OP, NFSD_CMD_THREADS_SET);
+	nla_put_u32(msg, CTRL_ATTR_OP, cmd);
 
 	ret = nl_send_auto(sock, msg);
 	if (ret < 0)
@@ -559,7 +561,7 @@ static int query_nfsd_nl_policy(struct nl_sock *sock)
 	nl_cb_err(cb, NL_CB_CUSTOM, error_handler, &ret);
 	nl_cb_set(cb, NL_CB_FINISH, NL_CB_CUSTOM, finish_handler, &ret);
 	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, ack_handler, &ret);
-	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, getpolicy_handler, NULL);
+	nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, getpolicy_handler, max_nlattr);
 
 	while (ret > 0)
 		nl_recvmsgs(sock, cb);
@@ -574,6 +576,12 @@ out:
 	return ret;
 }
 
+static int query_nfsd_nl_policy(struct nl_sock *sock)
+{
+	return query_nfsd_nl_cmd_policy(sock, NFSD_CMD_THREADS_SET,
+					&nfsd_threads_max_nlattr);
+}
+
 static void status_usage(void)
 {
 	printf("Usage: %s status\n", taskname);

-- 
2.55.0

[PATCH RFC nfs-utils v2 03/10] nfsdctl: share the listener nest parser

[Jeff Layton]

parse_listener_get() unpacks a NFSD_A_SERVER_SOCK_ADDR nest inline. It
also treats every top-level attribute as one of those nests. A
LISTENER_GET reply carries nothing else, so the loop is correct today. A
reply that mixes attributes would count the other attributes as
listeners.

Move the nest walk into parse_sock_nest(). Skip an attribute that is not
a listener. Stop at MAX_NFSD_SOCKETS.

No functional change.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 8733f0377208..dd815ac53489 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -273,6 +273,28 @@ static void parse_version_get(struct genlmsghdr *gnlh)
 	}
 }
 
+/* Unpack one NFSD_A_SERVER_SOCK_ADDR nest. */
+static void parse_sock_nest(struct nlattr *attr, struct server_socket *sock)
+{
+	struct nlattr *a;
+	char *res;
+	int i;
+
+	nla_for_each_nested(a, attr, i) {
+		switch (nla_type(a)) {
+		case NFSD_A_SOCK_TRANSPORT_NAME:
+			res = strncpy(sock->name, nla_data(a),
+				      MAX_CLASS_NAME_LEN);
+			res[MAX_CLASS_NAME_LEN - 1] = '\0'; // just to be sure
+			break;
+		case NFSD_A_SOCK_ADDR:
+			memcpy(&sock->ss, nla_data(a), sizeof(sock->ss));
+			break;
+		}
+		sock->active = true;
+	}
+}
+
 static void parse_listener_get(struct genlmsghdr *gnlh)
 {
 	struct nlattr *attr;
@@ -283,24 +305,11 @@ static void parse_listener_get(struct genlmsghdr *gnlh)
 
 	nla_for_each_attr(attr, genlmsg_attrdata(gnlh, 0),
 			  genlmsg_attrlen(gnlh, 0), rem) {
-		struct nlattr *a;
-		char *res;
-		int i;
-
-		nla_for_each_nested(a, attr, i) {
-			switch (nla_type(a)) {
-			case NFSD_A_SOCK_TRANSPORT_NAME:
-				res = strncpy(nfsd_sockets[idx].name, nla_data(a),
-					      MAX_CLASS_NAME_LEN);
-				res[MAX_CLASS_NAME_LEN - 1] = '\0'; // just to be sure
-				break;
-			case NFSD_A_SOCK_ADDR:
-				memcpy(&nfsd_sockets[idx].ss, nla_data(a),
-					sizeof(nfsd_sockets[idx].ss));
-				break;
-			}
-			nfsd_sockets[idx].active = true;
-		}
+		if (nla_type(attr) != NFSD_A_SERVER_SOCK_ADDR)
+			continue;
+		if (idx >= MAX_NFSD_SOCKETS)
+			break;
+		parse_sock_nest(attr, &nfsd_sockets[idx]);
 		++idx;
 	}
 	nfsd_socket_count = idx;

-- 
2.55.0

[PATCH RFC nfs-utils v2 04/10] nfsdctl: add rpcbind registration helpers

[Jeff Layton]

Add the code that calls the local rpcbind. The callers come next.

nfsd_rpcb_register() matches each listener against each program that the
kernel reported. It calls rpcb_set() for every pair.

nfsd_rpcb_unset_all() clears every nfsd entry with nfs_svc_unregister().
That helper calls rpcb_unset() with no netconfig, so one call covers
every netid.

Three rules match what the kernel did:

- nfsdctl registers the wildcard address with the listener's port. It
  does not register the address that the listener uses.
  __svc_rpcb_register4() and __svc_rpcb_register6() did the same.
- nfsdctl does not register an RDMA listener. svc_register() runs only
  from svc_setup_socket(), so an RDMA listener never reached rpcbind.
- nfsdctl does not register a program that carries the no-udp flag on
  udp or udp6. NFSv4 sets vs_need_cong_ctrl, so the kernel never
  advertised NFSv4 there.

A failed call writes a warning and does not stop the caller. NFSv4 does
not need rpcbind. A v2 or v3 client that already knows the port does not
need it either.

rpcb_set() and the netconfig database are TI-RPC only, so the
registration side sits under HAVE_LIBTIRPC with a warning stub, matching
getport.c and rpc_socket.c. nfsd_rpcb_unset_all() needs neither and
stays unconditional; nfs_svc_unregister() has a pmap_unset() variant.

struct server_socket moves to the new header, because both files read it.
The netlink spec generates nfsdctl.h, so the definition cannot live
there.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/Makefile.am |  10 +-
 utils/nfsdctl/nfsdctl.c   |  13 +--
 utils/nfsdctl/rpcbind.c   | 251 ++++++++++++++++++++++++++++++++++++++++++++++
 utils/nfsdctl/rpcbind.h   |  42 ++++++++
 4 files changed, 300 insertions(+), 16 deletions(-)

diff --git a/utils/nfsdctl/Makefile.am b/utils/nfsdctl/Makefile.am
index 89c7ecd6f30b..1390c43c855d 100644
--- a/utils/nfsdctl/Makefile.am
+++ b/utils/nfsdctl/Makefile.am
@@ -5,9 +5,11 @@ EXTRA_DIST      = $(man8_MANS)
 
 sbin_PROGRAMS	= nfsdctl
 
-noinst_HEADERS = nfsdctl.h
-nfsdctl_SOURCES = nfsdctl.c
-nfsdctl_CFLAGS = $(LIBNL3_CFLAGS) $(LIBNLGENL3_CFLAGS) $(LIBREADLINE_CFLAGS)
-nfsdctl_LDADD = ../../support/nfs/libnfs.la $(LIBNL3_LIBS) $(LIBNLGENL3_LIBS) $(LIBREADLINE_LIBS)
+noinst_HEADERS = nfsdctl.h rpcbind.h
+nfsdctl_SOURCES = nfsdctl.c rpcbind.c
+nfsdctl_CFLAGS = $(LIBNL3_CFLAGS) $(LIBNLGENL3_CFLAGS) $(LIBREADLINE_CFLAGS) \
+		 $(TIRPC_CFLAGS)
+nfsdctl_LDADD = ../../support/nfs/libnfs.la $(LIBNL3_LIBS) $(LIBNLGENL3_LIBS) \
+		$(LIBREADLINE_LIBS) $(LIBTIRPC)
 
 MAINTAINERCLEANFILES = Makefile.in
diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index dd815ac53489..860ea9c09ad2 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -46,6 +46,7 @@
 
 #include "nfslib.h"
 #include "nfsdctl.h"
+#include "rpcbind.h"
 #include "conffile.h"
 #include "xlog.h"
 
@@ -72,18 +73,6 @@ struct nfs_version {
 
 struct nfs_version nfsd_versions[MAX_NFS_VERSIONS];
 
-/*
- * All of the existing netids are short strings (3-4 chars), but let's allow
- * for up to 16.
- */
-#define MAX_CLASS_NAME_LEN	16
-
-struct server_socket {
-	struct sockaddr_storage	ss;
-	char name[MAX_CLASS_NAME_LEN];
-	bool active;
-};
-
 #define MAX_NFSD_SOCKETS		256
 
 int nfsd_socket_count;
diff --git a/utils/nfsdctl/rpcbind.c b/utils/nfsdctl/rpcbind.c
new file mode 100644
index 000000000000..23034097b485
--- /dev/null
+++ b/utils/nfsdctl/rpcbind.c
@@ -0,0 +1,251 @@
+/*
+ * rpcbind.c -- register nfsd listeners with the local rpcbind
+ *
+ * A kernel that accepts NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND makes no
+ * rpcbind call of its own, so this program makes them instead. The kernel
+ * used to do this under nfsd_mutex, where a slow rpcbind stalled every
+ * other NFSD netlink operation.
+ *
+ * lockd still registers NLM itself, so nothing here touches it.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <netinet/in.h>
+#include <string.h>
+#include <sys/socket.h>
+
+#include <rpc/rpc.h>
+
+#ifdef HAVE_LIBTIRPC
+#include <rpc/rpcb_clnt.h>
+#include <netconfig.h>
+#endif	/* HAVE_LIBTIRPC */
+
+#include "nfslib.h"
+#include "rpcmisc.h"
+#include "sockaddr.h"
+#include "xlog.h"
+#include "rpcbind.h"
+
+#ifdef USE_SYSTEM_NFSD_NETLINK_H
+#include <linux/nfsd_netlink.h>
+#else
+#include "nfsd_netlink.h"
+#endif
+
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x)		(sizeof(x) / sizeof((x)[0]))
+#endif
+
+/*
+ * Programs to clear at shutdown. There is no reply to read at that point,
+ * so clear a superset. rpcb_unset() of an absent entry does nothing.
+ */
+static const struct {
+	rpcprog_t	program;
+	rpcvers_t	version;
+} nfsd_rpcb_all[] = {
+	{ NFS_PROGRAM,		2 },
+	{ NFS_PROGRAM,		3 },
+	{ NFS_PROGRAM,		4 },
+	{ NFS_ACL_PROGRAM,	2 },
+	{ NFS_ACL_PROGRAM,	3 },
+};
+
+/**
+ * nfsd_rpcb_unset_all - remove every nfsd entry from the local rpcbind
+ *
+ * rpcb_unset() matches [program, version, netid] and takes no address, so
+ * one call clears every netid. This mirrors the svc_unregister() sweep that
+ * svc_rpcb_setup() used to run, which an sv_no_rpcbind serv no longer does.
+ */
+void nfsd_rpcb_unset_all(void)
+{
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(nfsd_rpcb_all); i++)
+		nfs_svc_unregister(nfsd_rpcb_all[i].program,
+				   nfsd_rpcb_all[i].version);
+}
+
+#ifdef HAVE_LIBTIRPC
+
+/*
+ * Map a transport name and address family to an rpcbind netid.
+ *
+ * Returns NULL for a transport that the kernel never registered. RDMA is
+ * the case that matters: svc_register() is only reached from
+ * svc_setup_socket(), so an RDMA listener was always absent from rpcbind.
+ */
+static const char *nfsd_rpcb_netid(const char *transport, int family)
+{
+	bool v6 = (family == AF_INET6);
+
+	if (!strcmp(transport, "tcp"))
+		return v6 ? "tcp6" : "tcp";
+	if (!strcmp(transport, "udp"))
+		return v6 ? "udp6" : "udp";
+	return NULL;
+}
+
+/*
+ * Build the address to register. The kernel registered the wildcard address
+ * with the listener's port, not the address the listener is bound to, so
+ * match that.
+ */
+static bool nfsd_rpcb_wildcard(const struct sockaddr_storage *ss,
+			       struct sockaddr_storage *out, socklen_t *len)
+{
+	memset(out, 0, sizeof(*out));
+
+	switch (ss->ss_family) {
+	case AF_INET: {
+		struct sockaddr_in *s4 = (struct sockaddr_in *)out;
+
+		s4->sin_family = AF_INET;
+		s4->sin_addr.s_addr = htonl(INADDR_ANY);
+		s4->sin_port = ((const struct sockaddr_in *)ss)->sin_port;
+		*len = sizeof(*s4);
+		return true;
+	}
+	case AF_INET6: {
+		struct sockaddr_in6 *s6 = (struct sockaddr_in6 *)out;
+
+		s6->sin6_family = AF_INET6;
+		s6->sin6_addr = in6addr_any;
+		s6->sin6_port = ((const struct sockaddr_in6 *)ss)->sin6_port;
+		*len = sizeof(*s6);
+		return true;
+	}
+	}
+	return false;
+}
+
+static bool nfsd_rpcb_set_one(rpcprog_t program, rpcvers_t version,
+			      const char *netid,
+			      const struct sockaddr_storage *ss)
+{
+	struct sockaddr_storage wild;
+	struct netconfig *nconf;
+	struct netbuf nb;
+	socklen_t len;
+	bool ok;
+
+	if (!nfsd_rpcb_wildcard(ss, &wild, &len))
+		return false;
+
+	nconf = getnetconfigent(netid);
+	if (!nconf) {
+		xlog(L_WARNING, "no netconfig entry for %s", netid);
+		return false;
+	}
+
+	nb.buf = &wild;
+	nb.len = len;
+	nb.maxlen = len;
+
+	ok = (rpcb_set(program, version, nconf, &nb) == TRUE);
+	freenetconfigent(nconf);
+	return ok;
+}
+
+/**
+ * nfsd_rpcb_register - register the listeners the kernel reported
+ * @listeners: listeners that came up, from the listener_set reply
+ * @nlisteners: number of entries in @listeners
+ * @progs: programs and versions the kernel would have registered
+ * @nprogs: number of entries in @progs
+ *
+ * A failure is not fatal. NFSv4 does not need rpcbind, and a v2/v3 client
+ * that already knows the port does not need it either, so an unadvertised
+ * server is better than no server.
+ */
+void nfsd_rpcb_register(const struct server_socket *listeners, int nlisteners,
+			const struct nfsd_rpcb_prog *progs, int nprogs)
+{
+	unsigned int failed = 0;
+	int i, j, k;
+
+	/*
+	 * Clear first. rpcbind cannot express "drop one of two TCP ports",
+	 * and the kernel no longer sweeps stale entries at serv creation.
+	 */
+	nfsd_rpcb_unset_all();
+
+	for (i = 0; i < nlisteners; i++) {
+		const struct server_socket *l = &listeners[i];
+		const char *netid;
+		bool dup = false;
+
+		netid = nfsd_rpcb_netid(l->name, l->ss.ss_family);
+		if (!netid)
+			continue;
+
+		/*
+		 * The address registered is the wildcard, so two listeners
+		 * that share a netid and port produce the same entry.
+		 */
+		for (k = 0; k < i; k++) {
+			const struct server_socket *o = &listeners[k];
+			const char *other;
+
+			other = nfsd_rpcb_netid(o->name, o->ss.ss_family);
+			if (other && !strcmp(other, netid) &&
+			    nfs_get_port((const struct sockaddr *)&o->ss) ==
+			    nfs_get_port((const struct sockaddr *)&l->ss)) {
+				dup = true;
+				break;
+			}
+		}
+		if (dup)
+			continue;
+
+		for (j = 0; j < nprogs; j++) {
+			const struct nfsd_rpcb_prog *p = &progs[j];
+
+			/*
+			 * NFSv4 needs congestion control, so the kernel
+			 * never advertised it over UDP.
+			 */
+			if ((p->flags & NFSD_RPCBIND_FLAGS_NO_UDP) &&
+			    !strncmp(netid, "udp", 3))
+				continue;
+
+			if (nfsd_rpcb_set_one(p->program, p->version, netid,
+					      &l->ss))
+				continue;
+
+			xlog(L_WARNING,
+			     "cannot register program %u version %u on %s port %u",
+			     p->program, p->version, netid,
+			     nfs_get_port((const struct sockaddr *)&l->ss));
+			failed++;
+		}
+	}
+
+	if (failed)
+		xlog(L_WARNING,
+		     "some rpcbind registrations failed. Clients that look up the server in rpcbind cannot find it.");
+}
+
+#else	/* !HAVE_LIBTIRPC */
+
+/*
+ * rpcb_set() and the netconfig database are TI-RPC only, so a build
+ * without it cannot own the registration. userspace_rpcbind_supported()
+ * answers false there and the kernel keeps doing the calls itself, which
+ * leaves this unreachable.
+ */
+void nfsd_rpcb_register(const struct server_socket *UNUSED(listeners),
+			int UNUSED(nlisteners),
+			const struct nfsd_rpcb_prog *UNUSED(progs),
+			int UNUSED(nprogs))
+{
+	xlog(L_WARNING, "built without TI-RPC: cannot register with rpcbind");
+}
+
+#endif	/* !HAVE_LIBTIRPC */
diff --git a/utils/nfsdctl/rpcbind.h b/utils/nfsdctl/rpcbind.h
new file mode 100644
index 000000000000..46145da6ed99
--- /dev/null
+++ b/utils/nfsdctl/rpcbind.h
@@ -0,0 +1,42 @@
+/*
+ * rpcbind.h -- register nfsd listeners with the local rpcbind
+ */
+
+#ifndef _UTILS_NFSDCTL_RPCBIND_H
+#define _UTILS_NFSDCTL_RPCBIND_H
+
+#include <stdbool.h>
+#include <stdint.h>
+#include <sys/socket.h>
+
+#ifndef NFS_PROGRAM
+#define NFS_PROGRAM		100003
+#endif
+#ifndef NFS_ACL_PROGRAM
+#define NFS_ACL_PROGRAM		100227
+#endif
+
+/*
+ * All of the existing netids are short strings (3-4 chars), but let's allow
+ * for up to 16.
+ */
+#define MAX_CLASS_NAME_LEN	16
+
+struct server_socket {
+	struct sockaddr_storage	ss;
+	char name[MAX_CLASS_NAME_LEN];
+	bool active;
+};
+
+/* One [program, version] that the kernel would have registered. */
+struct nfsd_rpcb_prog {
+	uint32_t	program;
+	uint32_t	version;
+	uint32_t	flags;
+};
+
+void nfsd_rpcb_unset_all(void);
+void nfsd_rpcb_register(const struct server_socket *listeners, int nlisteners,
+			const struct nfsd_rpcb_prog *progs, int nprogs);
+
+#endif /* _UTILS_NFSDCTL_RPCBIND_H */

-- 
2.55.0

[PATCH RFC nfs-utils v2 05/10] nfsdctl: register nfsd listeners with rpcbind

[Jeff Layton]

The kernel registered NFS and NFSACL under nfsd_mutex. Every call was
synchronous. A local rpcbind that never replied therefore blocked every
other NFSD netlink command, and tripped the hung-task watchdog.

A kernel that offers NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND makes no
rpcbind call of its own. It answers a listener_set that carries the flag
with the programs to register and the listeners that started. This
program now does three steps instead:

  1: send the flag
  2: read the reply
  3: register the listeners, where nothing holds a lock

Read the maximum attribute of LISTENER_SET to detect support. This adds
no configuration. An older kernel does not offer the attribute, so that
kernel keeps the registration. A build without TI-RPC has no rpcb_set(),
so it reports no support either and leaves the kernel in charge.

nfsdctl clears every nfsd entry before it registers. rpcbind matches an
unset on [program, version, netid] and takes no address, so an unset
cannot drop one of two TCP ports. The sweep also replaces the
svc_unregister() that svc_rpcb_setup() ran at serv creation. An
sv_no_rpcbind serv no longer reaches that call.

That sweep makes step 2 load-bearing, so register only once the reply
has been parsed. A reply that carries no rpcbind attributes would
otherwise clear every NFS and NFSACL entry and put nothing back, leaving
a running server that advertises nothing.

Abort if the flag does not fit in the message. Sending without it leaves
the kernel owning rpcbind while this program rewrites the same entries.

The reply names only the listeners that started. A request that failed
part way therefore registers what exists, not what the caller asked for.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.8    |  16 ++++++
 utils/nfsdctl/nfsdctl.adoc |  17 ++++++
 utils/nfsdctl/nfsdctl.c    | 125 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.8 b/utils/nfsdctl/nfsdctl.8
index 1f526e77bebf..d90f1eb96a41 100644
--- a/utils/nfsdctl/nfsdctl.8
+++ b/utils/nfsdctl/nfsdctl.8
@@ -337,6 +337,22 @@ interface to achieve the same goals.
 Most of the commands that query the NFS server can be run as an unprivileged
 user, but configuring the server typically requires an account with elevated
 privileges.
+.SS "rpcbind registration"
+.sp
+On a kernel that supports it, nfsdctl registers the NFS and NFSACL programs
+with the local rpcbind itself. The kernel made those calls while it held an
+internal mutex, where a slow rpcbind blocked every other NFSD netlink
+command. nfsdctl detects support and needs no configuration.
+.sp
+nfsdctl registers the listeners after it sets them, and it clears the
+entries when the last thread stops. A failed registration is not fatal. The
+server starts, but a client that looks the server up in rpcbind cannot find
+it. NFSv4 does not need rpcbind, and a v2 or v3 client that already knows
+the port does not need it either.
+.sp
+RDMA listeners are not registered, which matches what the kernel did.
+.sp
+lockd still registers NLM itself on every kernel.
 .SH "SEE ALSO"
 .sp
 nfs.conf(5), rpc.nfsd(8), rpc.mountd(8), exports(5), exportfs(8), rpc.rquotad(8), nfsstat(8), netconfig(5)
diff --git a/utils/nfsdctl/nfsdctl.adoc b/utils/nfsdctl/nfsdctl.adoc
index e85348e29ed8..bd21234b0d9d 100644
--- a/utils/nfsdctl/nfsdctl.adoc
+++ b/utils/nfsdctl/nfsdctl.adoc
@@ -176,6 +176,23 @@ Most of the commands that query the NFS server can be run as an unprivileged
 user, but configuring the server typically requires an account with elevated
 privileges.
 
+=== rpcbind registration
+
+On a kernel that supports it, nfsdctl registers the NFS and NFSACL programs
+with the local rpcbind itself. The kernel made those calls while it held an
+internal mutex, where a slow rpcbind blocked every other NFSD netlink
+command. nfsdctl detects support and needs no configuration.
+
+nfsdctl registers the listeners after it sets them, and it clears the
+entries when the last thread stops. A failed registration is not fatal. The
+server starts, but a client that looks the server up in rpcbind cannot find
+it. NFSv4 does not need rpcbind, and a v2 or v3 client that already knows
+the port does not need it either.
+
+RDMA listeners are not registered, which matches what the kernel did.
+
+lockd still registers NLM itself on every kernel.
+
 == SEE ALSO
 
 nfs.conf(5), rpc.nfsd(8), rpc.mountd(8), exports(5), exportfs(8), rpc.rquotad(8), nfsstat(8), netconfig(5)
diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 860ea9c09ad2..699947e4507f 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -59,6 +59,9 @@ static int nfsd_nl_family;
 /* The highest attribute index supported by NFSD_CMD_THREADS_SET on this kernel */
 int nfsd_threads_max_nlattr;
 
+/* The same for NFSD_CMD_LISTENER_SET */
+int nfsd_listener_max_nlattr;
+
 struct nfs_version {
 	uint8_t	major;
 	uint8_t	minor;
@@ -78,6 +81,15 @@ struct nfs_version nfsd_versions[MAX_NFS_VERSIONS];
 int nfsd_socket_count;
 struct server_socket nfsd_sockets[MAX_NFSD_SOCKETS];
 
+/* Programs the kernel reported in the last LISTENER_SET reply. */
+#define MAX_RPCB_PROGS			16
+
+static int nfsd_rpcb_prog_count;
+static struct nfsd_rpcb_prog nfsd_rpcb_progs[MAX_RPCB_PROGS];
+
+/* Did a LISTENER_SET reply actually reach parse_listener_set()? */
+static bool nfsd_rpcb_reply_seen;
+
 const char *taskname;
 
 static const struct option help_only_options[] = {
@@ -304,6 +316,57 @@ static void parse_listener_get(struct genlmsghdr *gnlh)
 	nfsd_socket_count = idx;
 }
 
+/*
+ * A LISTENER_SET that asked to own rpcbind answers with the programs to
+ * register and the listeners that came up.
+ */
+static void parse_listener_set(struct genlmsghdr *gnlh)
+{
+	struct nlattr *attr;
+	int rem, idx = 0;
+
+	memset(nfsd_sockets, '\0', sizeof(*nfsd_sockets) * MAX_NFSD_SOCKETS);
+	nfsd_rpcb_prog_count = 0;
+
+	nla_for_each_attr(attr, genlmsg_attrdata(gnlh, 0),
+			  genlmsg_attrlen(gnlh, 0), rem) {
+		struct nfsd_rpcb_prog *p;
+		struct nlattr *a;
+		int i;
+
+		switch (nla_type(attr)) {
+		case NFSD_A_SERVER_SOCK_ADDR:
+			if (idx >= MAX_NFSD_SOCKETS)
+				break;
+			parse_sock_nest(attr, &nfsd_sockets[idx]);
+			++idx;
+			break;
+		case NFSD_A_SERVER_SOCK_RPCBIND:
+			if (nfsd_rpcb_prog_count >= MAX_RPCB_PROGS)
+				break;
+			p = &nfsd_rpcb_progs[nfsd_rpcb_prog_count];
+			memset(p, '\0', sizeof(*p));
+			nla_for_each_nested(a, attr, i) {
+				switch (nla_type(a)) {
+				case NFSD_A_RPCBIND_PROGRAM:
+					p->program = nla_get_u32(a);
+					break;
+				case NFSD_A_RPCBIND_VERSION:
+					p->version = nla_get_u32(a);
+					break;
+				case NFSD_A_RPCBIND_FLAGS:
+					p->flags = nla_get_u32(a);
+					break;
+				}
+			}
+			++nfsd_rpcb_prog_count;
+			break;
+		}
+	}
+	nfsd_socket_count = idx;
+	nfsd_rpcb_reply_seen = true;
+}
+
 static void parse_threads_get(struct genlmsghdr *gnlh)
 {
 	struct nlattr *attr;
@@ -405,6 +468,9 @@ static int recv_handler(struct nl_msg *msg, void *arg)
 	case NFSD_CMD_LISTENER_GET:
 		parse_listener_get(gnlh);
 		break;
+	case NFSD_CMD_LISTENER_SET:
+		parse_listener_set(gnlh);
+		break;
 	case NFSD_CMD_POOL_MODE_GET:
 		parse_pool_mode_get(gnlh);
 		break;
@@ -576,8 +642,29 @@ out:
 
 static int query_nfsd_nl_policy(struct nl_sock *sock)
 {
-	return query_nfsd_nl_cmd_policy(sock, NFSD_CMD_THREADS_SET,
-					&nfsd_threads_max_nlattr);
+	int ret;
+
+	ret = query_nfsd_nl_cmd_policy(sock, NFSD_CMD_THREADS_SET,
+				       &nfsd_threads_max_nlattr);
+	if (ret)
+		return ret;
+
+	/*
+	 * A kernel that offers NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND expects
+	 * this program to register the listeners itself.
+	 */
+	return query_nfsd_nl_cmd_policy(sock, NFSD_CMD_LISTENER_SET,
+					&nfsd_listener_max_nlattr);
+}
+
+static bool userspace_rpcbind_supported(void)
+{
+#ifdef HAVE_LIBTIRPC
+	return nfsd_listener_max_nlattr >= NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND;
+#else
+	/* No rpcb_set() here, so leave the registration to the kernel. */
+	return false;
+#endif
 }
 
 static void status_usage(void)
@@ -1340,12 +1427,15 @@ out_inval:
 
 static int set_listeners(struct nl_sock *sock)
 {
+	bool userspace_rpcbind = userspace_rpcbind_supported();
 	struct genlmsghdr *ghdr;
 	struct nlmsghdr *nlh;
 	struct nl_msg *msg;
 	struct nl_cb *cb;
 	int i, ret;
 
+	nfsd_rpcb_reply_seen = false;
+
 	if (!nfsd_nl_family_setup(sock))
 		return 1;
 
@@ -1382,6 +1472,21 @@ static int set_listeners(struct nl_sock *sock)
 		nla_nest_end(msg, a);
 	}
 
+	/*
+	 * Take rpcbind registration off the kernel. It made those calls
+	 * under nfsd_mutex, where a slow rpcbind stalled every other NFSD
+	 * netlink operation.
+	 *
+	 * Losing this flag would leave the kernel owning rpcbind while this
+	 * program went on to rewrite the entries, so do not send without it.
+	 */
+	if (userspace_rpcbind &&
+	    nla_put_flag(msg, NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND)) {
+		xlog(L_ERROR, "Unable to request rpcbind ownership");
+		ret = 1;
+		goto out;
+	}
+
 	ghdr = nlmsg_data(nlh);
 	ghdr->cmd = NFSD_CMD_LISTENER_SET;
 
@@ -1410,6 +1515,22 @@ static int set_listeners(struct nl_sock *sock)
 		xlog(L_ERROR, "Error: %s", strerror(-ret));
 		ret = 1;
 	}
+
+	/*
+	 * recv_handler() filled nfsd_sockets and nfsd_rpcb_progs from the
+	 * reply, which names only the listeners that came up. Without that
+	 * reply there is nothing to register, and registering anyway would
+	 * clear the entries the kernel installed and put nothing back.
+	 */
+	if (!ret && userspace_rpcbind) {
+		if (nfsd_rpcb_reply_seen)
+			nfsd_rpcb_register(nfsd_sockets, nfsd_socket_count,
+					   nfsd_rpcb_progs,
+					   nfsd_rpcb_prog_count);
+		else
+			xlog(L_WARNING,
+			     "no rpcbind info in the listener_set reply. Leaving the rpcbind registrations alone.");
+	}
 out_cb:
 	nl_cb_put(cb);
 out:

-- 
2.55.0

[PATCH RFC nfs-utils v2 06/10] nfsdctl: clear rpcbind entries when the serv is torn down

[Jeff Layton]

nfsd destroys the serv when the last thread stops. The kernel used to
clear the rpcbind entries on that path, through svc_rpcb_cleanup(). A
serv that let this program own rpcbind does not clear them. The entries
would then outlive the server and name a port that nothing listens on.

Clear them from "nfsdctl threads 0". nfs-server.service runs that command
to stop the server. autostart_func() tears the serv back down the same
way when a listener fails to start, so sweep there too.

zero_threads was true when any one pool argument was zero. That is the
wrong test to hang the sweep off: "nfsdctl threads 8 0" empties pool 1,
keeps the serv, and would have cleared every entry under a running
server. Sum the pool counts instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 699947e4507f..4cc33672a41f 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -836,7 +836,7 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
 	uint8_t cmd = NFSD_CMD_THREADS_GET;
 	int *pool_threads = NULL;
 	int minthreads = -1;
-	int opt, pools = 0;
+	int opt, pools = 0, ret;
 	uuid_t fh_key;
 	bool zero_threads = false;
 
@@ -877,7 +877,7 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
 
 	if (optind < argc) {
 		char **targv = &argv[optind];
-		int i;
+		int i, total = 0;
 
 		pools = argc - optind;
 		pool_threads = alloca(pools * sizeof(*pool_threads));
@@ -893,13 +893,18 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
 			}
 
 			pool_threads[i] = strtol(targv[i], &endptr, 0);
-			if (!pool_threads[i])
-				zero_threads = true;
+			total += pool_threads[i];
 			if (!endptr || *endptr != '\0') {
 				xlog(L_ERROR, "Invalid threads value %s.", argv[1]);
 				return 1;
 			}
 		}
+
+		/*
+		 * The serv only goes away when no pool has a thread left.
+		 * "threads 8 0" empties one pool and keeps the server up.
+		 */
+		zero_threads = (total == 0);
 	}
 	/* check if there are active listeners added */
 	if (fetch_current_listeners(sock)) {
@@ -912,14 +917,25 @@ static int threads_func(struct nl_sock *sock, int argc, char **argv)
 			 * listener. If we ever add functionality to remove
 			 * listeners on an active server, we need to revisit this.
 			 */
+			if (userspace_rpcbind_supported())
+				nfsd_rpcb_unset_all();
 			return 0;
 		}
 		xlog(L_ERROR, "No active listeners added, not starting threads");
 		return 1;
 	}
 
-	return threads_doit(sock, cmd, 0, 0, pools, pool_threads, NULL,
+	ret = threads_doit(sock, cmd, 0, 0, pools, pool_threads, NULL,
 				minthreads, fh_key);
+
+	/*
+	 * The last thread destroys the serv, and a serv that never called
+	 * rpcb_create_local() leaves its entries behind.
+	 */
+	if (!ret && zero_threads && userspace_rpcbind_supported())
+		nfsd_rpcb_unset_all();
+
+	return ret;
 }
 
 /*
@@ -2041,6 +2057,13 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 
 	ret = threads_doit(sock, NFSD_CMD_THREADS_SET, grace, lease, pools,
 			   threads, scope, minthreads, fh_key);
+
+	/*
+	 * That teardown destroys the serv, and a serv that never called
+	 * rpcb_create_local() leaves its entries behind.
+	 */
+	if (!ret && !pools && userspace_rpcbind_supported())
+		nfsd_rpcb_unset_all();
 out:
 	free(threads);
 	return ret;

-- 
2.55.0

[PATCH RFC nfs-utils v2 07/10] nfsdctl: add missing break in update_listeners()

[Jeff Layton]

The AF_INET arm fell through into AF_INET6, so a v4 listener that
matched on port and address was then re-tested as a sockaddr_in6 and
skipped. -Wimplicit-fallthrough is not enabled, so nothing caught it.

Terminate every arm. The AF_INET6 fallthrough was harmless, since
"default:" is empty, but it is a trap for the next case added.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 4cc33672a41f..ce4da2769e5a 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -1410,13 +1410,15 @@ static int update_listeners(const char *str)
 				if (r4->sin_port != l4->sin_port ||
 				    memcmp(&r4->sin_addr, &l4->sin_addr, sizeof(l4->sin_addr)))
 					continue;
+				break;
 			case AF_INET6:
 				if (r6->sin6_port != l6->sin6_port ||
 				    memcmp(&r6->sin6_addr, &l6->sin6_addr, sizeof(l6->sin6_addr)) ||
 				    !ipv6_is_enabled())
 					continue;
+				break;
 			default:
-
+				break;
 			}
 			sock->active = (sign == '+');
 			found = true;

-- 
2.55.0

[PATCH RFC nfs-utils v2 08/10] nfsdctl: fix heap overflow in autostart_func() listener cleanup

[Jeff Layton]

The listener-failure cleanup forced pools to 0 before sizing the array:

	pools = thread_str ? thread_str->cnt : 1;
	if (failed_listeners)
		pools = 0;
	threads = calloc(pools, sizeof(int));
	...
	if (thread_str)
		TAILQ_FOREACH(n, &(thread_str->fields), link)
			threads[idx++] = strtol(n->field, &endptr, 0);

The TAILQ walk is driven by thread_str, not by pools, so it writes
thread_str->cnt ints into a calloc(0) allocation. Any /etc/nfs.conf with
a [nfsd] threads setting overflows the heap once a listener fails to
come up.

Size the array from the list and clamp pools to 0 just before
threads_doit(), which is the only consumer. The netlink message is
unchanged.

Also switch the hash_fh_key_file() failure to goto out; it returned
directly and leaked threads.

Fixes: 8b02f0d5590e ("nfsdctl: cleanup listeners if some failed")
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index ce4da2769e5a..a2b5dea75dac 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -2014,12 +2014,6 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 	thread_str = conf_get_list("nfsd", "threads");
 	pools = thread_str ? thread_str->cnt : 1;
 
-	/* if we fail to start one or more listeners, then cleanup by
-	 * starting 0 knfsd threads
-	 */
-	if (failed_listeners)
-		pools = 0;
-
 	threads = calloc(pools, sizeof(int));
 	if (!threads)
 		return -ENOMEM;
@@ -2045,7 +2039,7 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 	if (fh_key_file) {
 		ret = hash_fh_key_file(fh_key_file, fh_key);
 		if (ret)
-			return ret;
+			goto out;
 	}
 
 	lease = conf_get_num("nfsd", "lease-time", 0);
@@ -2057,6 +2051,12 @@ static int autostart_func(struct nl_sock *sock, int argc, char ** argv)
 		minthreads = -1;
 	}
 
+	/* if we fail to start one or more listeners, then cleanup by
+	 * starting 0 knfsd threads
+	 */
+	if (failed_listeners)
+		pools = 0;
+
 	ret = threads_doit(sock, NFSD_CMD_THREADS_SET, grace, lease, pools,
 			   threads, scope, minthreads, fh_key);
 

-- 
2.55.0

[PATCH RFC nfs-utils v2 09/10] nfsdctl: reject a listener insertion when the table is full

[Jeff Layton]

parse_listener_get() stops at MAX_NFSD_SOCKETS and publishes
nfsd_socket_count = MAX_NFSD_SOCKETS. update_listeners() then scans the
whole table for a match, finds none because every slot is taken, and
forms &nfsd_sockets[MAX_NFSD_SOCKETS] for the insert. The memcpy() of
res->ai_addr writes one element past the array.

Reachable only with 256 listeners already up, and the count comes from
the kernel rather than the network, so this is hardening.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index a2b5dea75dac..7464baad279f 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -1425,8 +1425,16 @@ static int update_listeners(const char *str)
 			break;
 		}
 		if (!found && sign == '+') {
-			struct server_socket *sock = &nfsd_sockets[nfsd_socket_count];
+			struct server_socket *sock;
 
+			if (nfsd_socket_count >= MAX_NFSD_SOCKETS) {
+				xlog(L_ERROR, "Too many listeners, max is %d",
+				     MAX_NFSD_SOCKETS);
+				free(buf);
+				return -ENOSPC;
+			}
+
+			sock = &nfsd_sockets[nfsd_socket_count];
 			memcpy(&sock->ss, res->ai_addr, res->ai_addrlen);
 			strncpy(sock->name, netid, MAX_CLASS_NAME_LEN);
 			sock->name[MAX_CLASS_NAME_LEN - 1] = '\0';

-- 
2.55.0

[PATCH RFC nfs-utils v2 10/10] nfsdctl: check the netlink attribute puts in set_listeners()

[Jeff Layton]

nla_nest_start() only reserves a 4-byte header, so it still succeeds on
a message that has no room for the 128-byte sockaddr that follows. The
puts inside the nest went unchecked, so an over-full message was sent
with silently truncated listener nests. The kernel then rejects it with
EINVAL, which says nothing about what went wrong.

MAX_NFSD_SOCKETS is 256 and nlmsg_alloc() defaults to one page, so
roughly 28 listeners overflow.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 utils/nfsdctl/nfsdctl.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/utils/nfsdctl/nfsdctl.c b/utils/nfsdctl/nfsdctl.c
index 7464baad279f..505b38d695ab 100644
--- a/utils/nfsdctl/nfsdctl.c
+++ b/utils/nfsdctl/nfsdctl.c
@@ -1493,8 +1493,14 @@ static int set_listeners(struct nl_sock *sock)
 			goto out;
 		}
 
-		nla_put(msg, NFSD_A_SOCK_ADDR, sizeof(sock->ss), &sock->ss);
-		nla_put_string(msg, NFSD_A_SOCK_TRANSPORT_NAME, sock->name);
+		if (nla_put(msg, NFSD_A_SOCK_ADDR, sizeof(sock->ss),
+			    &sock->ss) ||
+		    nla_put_string(msg, NFSD_A_SOCK_TRANSPORT_NAME,
+				   sock->name)) {
+			xlog(L_ERROR, "Too many listeners for one netlink message");
+			ret = 1;
+			goto out;
+		}
 		nla_nest_end(msg, a);
 	}
 

-- 
2.55.0

Re: [PATCH RFC nfs-utils v2 00/10] nfsdctl: allow it to handle rpcbind registration and deregistration

[Jeff Layton]

On Mon, 2026-09-14 at 12:00 -0400, Jeff Layton wrote:
> This version fixes some problems that Sashiko flagged in review.
> 
> This patchset adds support for NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND,
> which tells the kernel to not do rpcbind registration and to instead
> allow userland to handle it. After setting listeners, nfsdctl will then
> call out to rpcbind using the normal libtirpc client interfaces.
> 
> This is a companion set to the kernel set I recently posted (and which
> also should have been marked RFC).
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> Changes in v2:
> - Fixes for bugs v1 introduced are folded into the patches that
>   introduced them. Three pre-existing bugs found along the way are
>   separate patches at the end, so the set is 10 patches instead of 7.
> - "register nfsd listeners with rpcbind": register only once the
>   listener_set reply has been parsed. On a reply carrying no rpcbind
>   attributes the sweep cleared every NFS and NFSACL entry and put
>   nothing back, leaving a running server advertising nothing. Also
>   abort rather than send a listener_set that lost the ownership flag.
> - "clear rpcbind entries when the serv is torn down" (was "... when the
>   last thread stops"): also sweep on the autostart teardown path.
> - New, pre-existing: fix a heap overflow in autostart_func(). A failed
>   listener set zeroed the pool count before the thread array was sized,
>   but the array is filled from the nfs.conf list, so any [nfsd] threads
>   setting wrote past the allocation.
> - New, pre-existing: reject a listener insertion when the table is full,
>   instead of writing one element past nfsd_sockets[].
> - New, pre-existing: check the nla_put() calls in set_listeners().
> - Link to v1: https://lore.kernel.org/r/20260910-nfsdctl-rpcb-v1-0-4bba1e7a3a43@kernel.org
> 
> ---
> Jeff Layton (10):
>       nfsdctl: sync nfsd_netlink.h with the kernel
>       nfsdctl: probe the policy of any command
>       nfsdctl: share the listener nest parser
>       nfsdctl: add rpcbind registration helpers
>       nfsdctl: register nfsd listeners with rpcbind
>       nfsdctl: clear rpcbind entries when the serv is torn down
>       nfsdctl: add missing break in update_listeners()
>       nfsdctl: fix heap overflow in autostart_func() listener cleanup
>       nfsdctl: reject a listener insertion when the table is full
>       nfsdctl: check the netlink attribute puts in set_listeners()
> 
>  configure.ac                   |   2 +-
>  support/include/nfsd_netlink.h |  18 +++
>  utils/nfsdctl/Makefile.am      |  10 +-
>  utils/nfsdctl/nfsdctl.8        |  16 +++
>  utils/nfsdctl/nfsdctl.adoc     |  17 +++
>  utils/nfsdctl/nfsdctl.c        | 256 +++++++++++++++++++++++++++++++++--------
>  utils/nfsdctl/rpcbind.c        | 251 ++++++++++++++++++++++++++++++++++++++++
>  utils/nfsdctl/rpcbind.h        |  42 +++++++
>  8 files changed, 562 insertions(+), 50 deletions(-)
> ---
> base-commit: 630393e9c73667b3fda6a7507cb5171ec0ec38dc
> change-id: 20260908-nfsdctl-rpcb-adc5a1fc090b
> 
> Best regards,

Forgot to remove the "RFC" from the subject line for this one. Consider
this an "official" submission.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH RFC nfs-utils v2 00/10] nfsdctl: allow it to handle rpcbind registration and deregistration

[Steve Dickson]

On 9/14/26 12:00 PM, Jeff Layton wrote:
> This version fixes some problems that Sashiko flagged in review.
> 
> This patchset adds support for NFSD_A_SERVER_SOCK_USERSPACE_RPCBIND,
> which tells the kernel to not do rpcbind registration and to instead
> allow userland to handle it. After setting listeners, nfsdctl will then
> call out to rpcbind using the normal libtirpc client interfaces.
> 
> This is a companion set to the kernel set I recently posted (and which
> also should have been marked RFC).
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
Committed... (tag: nfs-utils-2-9-3-rc7)

steved.> ---
> Changes in v2:
> - Fixes for bugs v1 introduced are folded into the patches that
>    introduced them. Three pre-existing bugs found along the way are
>    separate patches at the end, so the set is 10 patches instead of 7.
> - "register nfsd listeners with rpcbind": register only once the
>    listener_set reply has been parsed. On a reply carrying no rpcbind
>    attributes the sweep cleared every NFS and NFSACL entry and put
>    nothing back, leaving a running server advertising nothing. Also
>    abort rather than send a listener_set that lost the ownership flag.
> - "clear rpcbind entries when the serv is torn down" (was "... when the
>    last thread stops"): also sweep on the autostart teardown path.
> - New, pre-existing: fix a heap overflow in autostart_func(). A failed
>    listener set zeroed the pool count before the thread array was sized,
>    but the array is filled from the nfs.conf list, so any [nfsd] threads
>    setting wrote past the allocation.
> - New, pre-existing: reject a listener insertion when the table is full,
>    instead of writing one element past nfsd_sockets[].
> - New, pre-existing: check the nla_put() calls in set_listeners().
> - Link to v1: https://lore.kernel.org/r/20260910-nfsdctl-rpcb-v1-0-4bba1e7a3a43@kernel.org
> 
> ---
> Jeff Layton (10):
>        nfsdctl: sync nfsd_netlink.h with the kernel
>        nfsdctl: probe the policy of any command
>        nfsdctl: share the listener nest parser
>        nfsdctl: add rpcbind registration helpers
>        nfsdctl: register nfsd listeners with rpcbind
>        nfsdctl: clear rpcbind entries when the serv is torn down
>        nfsdctl: add missing break in update_listeners()
>        nfsdctl: fix heap overflow in autostart_func() listener cleanup
>        nfsdctl: reject a listener insertion when the table is full
>        nfsdctl: check the netlink attribute puts in set_listeners()
> 
>   configure.ac                   |   2 +-
>   support/include/nfsd_netlink.h |  18 +++
>   utils/nfsdctl/Makefile.am      |  10 +-
>   utils/nfsdctl/nfsdctl.8        |  16 +++
>   utils/nfsdctl/nfsdctl.adoc     |  17 +++
>   utils/nfsdctl/nfsdctl.c        | 256 +++++++++++++++++++++++++++++++++--------
>   utils/nfsdctl/rpcbind.c        | 251 ++++++++++++++++++++++++++++++++++++++++
>   utils/nfsdctl/rpcbind.h        |  42 +++++++
>   8 files changed, 562 insertions(+), 50 deletions(-)
> ---
> base-commit: 630393e9c73667b3fda6a7507cb5171ec0ec38dc
> change-id: 20260908-nfsdctl-rpcb-adc5a1fc090b
> 
> Best regards,