On Thu, 10 Sep 2026, Jeff Layton wrote:
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -2273,12 +2396,28 @@ int nfsd_nl_listener_set_doit(struct sk_buff *skb,
> struct genl_info *info)
> + /*
> + * Build the reply before the serv can go away, and only on success.
> + * A caller that got an errno has nothing to register.
> + */
> + if (!err && userspace_rpcbind) {
> + rskb = nfsd_nl_listener_set_msg(info, net, serv);
> + if (IS_ERR(rskb)) {
> + err = PTR_ERR(rskb);
> + rskb = NULL;
> + }
> + }
The second sentence of the comment is not true after a partial
failure. Take a request with userspace-rpcbind carrying {tcp:2049,
tcp:<port already in use>}. The creation loop continues past the
second failure and keeps only the last errno. The 2049 xprt is on
sv_permsocks, so the serv survives the list_empty() check below, but
this gate skips the reply and the caller sees -EADDRINUSE. nfsd is
now serving on 2049 with nothing registered in rpcbind, and the caller
was never told which listeners came up or which programs to register.
The same end state results when nfsd_nl_listener_set_msg() itself
fails with -ENOMEM.
In kernel-owned mode the survivor would already have been registered
inside svc_xprt_create_from_sa(), so this is a behavioral gap specific
to the new mode. Either tear down the listeners that were created when
any of them fails, or send the reply whenever at least one listener
exists on the serv, regardless of err. The second keeps the existing
partial-success semantics; the caller can then act on what it got.
> +static size_t nfsd_nl_listener_set_msgsize(struct svc_serv *serv)
> +{
> + size_t size = GENL_HDRLEN + /* genlmsg_iput() */
> + nla_total_size(0); /* userspace-rpcbind */
genlmsg_new() already adds GENL_HDRLEN via genlmsg_total_size(), so
this term double-counts it. Harmless, but the comment will mislead the
next person sizing a reply. Pass only the attribute payload here.
> + spin_lock_bh(&serv->sv_lock);
> + list_for_each_entry(xprt, &serv->sv_permsocks, xpt_list) {
> + struct nlattr *attr;
> +
> + if (!test_bit(XPT_RPCB_UNREG, &xprt->xpt_flags))
> + continue;
> +
> + attr = nla_nest_start(skb, NFSD_A_SERVER_SOCK_ADDR);
> + if (!attr) {
> + err = -EMSGSIZE;
> + goto err_serv_unlock;
> + }
> +
> + if (nla_put_string(skb, NFSD_A_SOCK_TRANSPORT_NAME,
> + xprt->xpt_class->xcl_name) ||
> + nla_put(skb, NFSD_A_SOCK_ADDR,
> + sizeof(struct sockaddr_storage),
> + &xprt->xpt_local)) {
> + err = -EMSGSIZE;
> + goto err_serv_unlock;
> + }
> +
> + nla_nest_end(skb, attr);
> + }
This is the same nest emission as the loop in
nfsd_nl_listener_get_doit(), apart from the XPT_RPCB_UNREG filter and
the errno (that one returns -EINVAL). A small helper that emits one
addr nest for an xprt, called from both loops, would keep the two from
drifting, with the size accounting above as the third place to keep
in step.
Also, as noted on patch 2, this filter selects TCP and UDP only; the
spec doc should match.
> --- a/fs/nfsd/nfssvc.c
> +++ b/fs/nfsd/nfssvc.c
> @@ -859,12 +842,44 @@ nfsd_acl_init_request(struct svc_rqst *rqstp,
> +bool nfsd_version_registerable(struct net *net,
> + const struct svc_program *progp, u32 version)
> +{
> + struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +
> + if (version >= progp->pg_nvers || !progp->pg_vers[version])
> + return false;
> +
> + /* nfslocalio is hidden and never reaches rpcbind. */
> + if (progp->pg_vers[version]->vs_hidden)
> + return false;
> +
> +#if defined(CONFIG_NFSD_V2_ACL) || defined(CONFIG_NFSD_V3_ACL)
> + if (progp->pg_prog == NFS_ACL_PROGRAM &&
> + !nfsd_support_acl_version(version))
> + return false;
> +#endif
The ACL block is redundant with the first test. The ACL program has
pg_nvers = NFSD_ACL_NRVERS and pg_vers = nfsd_acl_version, whose
slots 0 and 1 and any compiled-out version are NULL, and
nfsd_support_acl_version() checks exactly that range and that array.
Dropping the block removes the function's only program-number special
case and the #if with it.
--
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
