Linux NFS development
 help / color / mirror / Atom feed
* [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces
@ 2026-09-09 12:56 Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 1/5] mountd: factor out the per-path export attribute computation Jeff Layton
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

Mantas reported a problem with his configuration that was using crossmnt
and autofs to export a number of btrfs filesystems. Some analysis with
Claude also uncovered a few other bugs. This patchset is the result.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (5):
      mountd: factor out the per-path export attribute computation
      mountd: handle unmountable paths and junctions in the netlink downcall
      mountd: answer requests the kernel rejects on the netlink downcall
      mountd: don't leak the parent export's fsid onto crossmnt submounts
      mountd: retry unresolvable fsid lookups on the netlink downcall

 support/export/cache.c | 1046 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 757 insertions(+), 289 deletions(-)
---
base-commit: 630393e9c73667b3fda6a7507cb5171ec0ec38dc
change-id: 20260908-nl-crossmnt-85584b71c721

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH nfs-utils 1/5] mountd: factor out the per-path export attribute computation
  2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
@ 2026-09-09 12:56 ` Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 2/5] mountd: handle unmountable paths and junctions in the netlink downcall Jeff Layton
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

dump_to_cache() adjusts the export flags, fsid and uuid before handing
them to the kernel, because the upcall path may be a crossmnt submount
rather than the exported path itself.  Pull that into
export_attrs_build() so the netlink downcall can use it too.

No functional change.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 support/export/cache.c | 143 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 88 insertions(+), 55 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index 059f48a7069f..e8b13409ad7a 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -1072,6 +1072,86 @@ static void write_xprtsec(char **bp, int *blen, struct exportent *ep)
 		qword_addint(bp, blen, p->info->number);
 }
 
+static int can_reexport_via_fsidnum(struct exportent *exp, struct statfs *st)
+{
+	if (st->f_type != 0x6969 /* NFS_SUPER_MAGIC */)
+		return 0;
+
+	return exp->e_reexport == REEXP_PREDEFINED_FSIDNUM ||
+	       exp->e_reexport == REEXP_AUTO_FSIDNUM;
+}
+
+/* What to hand the kernel for one (path, export) pair */
+struct export_attrs {
+	int		flags;
+	uint32_t	fsidnum;
+	int		sec_mask;	/* mask for the per-flavor flags */
+	int		sec_extra;	/* extra per-flavor flags */
+	char		uuid[16];
+	bool		have_uuid;
+};
+
+/*
+ * An upcall path may be a submount below the exported one, when the export
+ * is marked crossmnt.  Such a submount is a filesystem in its own right, so
+ * it must not inherit the parent's fsid= or uuid= - if it does, both end up
+ * claiming the same filehandles and the client sees ESTALE.
+ *
+ * Returns 0, or -1 with errno set if @path cannot be exported at all.
+ */
+static int export_attrs_build(struct export_attrs *ea, char *path,
+			      struct exportent *exp)
+{
+	int different_fs = strcmp(path, exp->e_path) != 0;
+	int flag_mask = different_fs ? ~NFSEXP_FSID : ~0;
+	int do_fsidnum = 0;
+
+	memset(ea, 0, sizeof(*ea));
+	ea->fsidnum = exp->e_fsid;
+
+	if (different_fs) {
+		struct statfs st;
+
+		if (nfsd_path_statfs(path, &st)) {
+			xlog(L_WARNING, "unable to statfs %s", path);
+			errno = EINVAL;
+			return -1;
+		}
+
+		/* A re-exported submount gets an fsid= of its own instead */
+		if (can_reexport_via_fsidnum(exp, &st)) {
+			do_fsidnum = 1;
+			flag_mask = ~0;
+		}
+	}
+
+	if (do_fsidnum) {
+		uint32_t search_fsidnum = 0;
+
+		if (exp->e_reexport != REEXP_NONE &&
+		    reexpdb_fsidnum_by_path(path, &search_fsidnum,
+			    exp->e_reexport == REEXP_AUTO_FSIDNUM) == 0) {
+			errno = EINVAL;
+			return -1;
+		}
+		ea->fsidnum = search_fsidnum;
+		ea->flags = exp->e_flags | NFSEXP_FSID;
+		ea->sec_extra = NFSEXP_FSID;
+	} else {
+		ea->flags = exp->e_flags & flag_mask;
+	}
+	ea->sec_mask = flag_mask;
+
+	if (exp->e_uuid && !different_fs) {
+		get_uuid(exp->e_uuid, 16, ea->uuid);
+		ea->have_uuid = true;
+	} else if ((exp->e_flags & flag_mask & NFSEXP_FSID) == 0) {
+		ea->have_uuid = uuid_by_path(path, 0, 16, ea->uuid);
+	}
+
+	return 0;
+}
+
 /*
  * Netlink-based svc_export cache support.
  *
@@ -2501,15 +2581,6 @@ static void cache_sunrpc_nl_process(void)
 		cache_nl_process_unix_gid();
 }
 
-static int can_reexport_via_fsidnum(struct exportent *exp, struct statfs *st)
-{
-	if (st->f_type != 0x6969 /* NFS_SUPER_MAGIC */)
-		return 0;
-
-	return exp->e_reexport == REEXP_PREDEFINED_FSIDNUM ||
-	       exp->e_reexport == REEXP_AUTO_FSIDNUM;
-}
-
 static int dump_to_cache(int f, char *buf, int blen, char *domain,
 			 char *path, struct exportent *exp, int ttl)
 {
@@ -2524,60 +2595,22 @@ static int dump_to_cache(int f, char *buf, int blen, char *domain,
 	qword_add(&bp, &blen, domain);
 	qword_add(&bp, &blen, path);
 	if (exp) {
-		int different_fs = strcmp(path, exp->e_path) != 0;
-		int flag_mask = different_fs ? ~NFSEXP_FSID : ~0;
-		int rc, do_fsidnum = 0;
-		uint32_t fsidnum = exp->e_fsid;
-
-		if (different_fs) {
-			struct statfs st;
-
-			rc = nfsd_path_statfs(path, &st);
-			if (rc) {
-				xlog(L_WARNING, "unable to statfs %s", path);
-				errno = EINVAL;
-				return -1;
-			}
+		struct export_attrs ea;
 
-			if (can_reexport_via_fsidnum(exp, &st)) {
-				do_fsidnum = 1;
-				flag_mask = ~0;
-			}
-		}
+		if (export_attrs_build(&ea, path, exp) < 0)
+			return -1;
 
 		qword_adduint(&bp, &blen, now + exp->e_ttl);
-
-		if (do_fsidnum) {
-			uint32_t search_fsidnum = 0;
-			if (exp->e_reexport != REEXP_NONE && reexpdb_fsidnum_by_path(path, &search_fsidnum,
-			    exp->e_reexport == REEXP_AUTO_FSIDNUM) == 0) {
-				errno = EINVAL;
-				return -1;
-			}
-			fsidnum = search_fsidnum;
-			qword_addint(&bp, &blen, exp->e_flags | NFSEXP_FSID);
-		} else {
-			qword_addint(&bp, &blen, exp->e_flags & flag_mask);
-		}
-
+		qword_addint(&bp, &blen, ea.flags);
 		qword_addint(&bp, &blen, exp->e_anonuid);
 		qword_addint(&bp, &blen, exp->e_anongid);
-		qword_addint(&bp, &blen, fsidnum);
+		qword_addint(&bp, &blen, ea.fsidnum);
 
 		write_fsloc(&bp, &blen, exp);
-		write_secinfo(&bp, &blen, exp, flag_mask, do_fsidnum ? NFSEXP_FSID : 0);
-		if (exp->e_uuid == NULL || different_fs) {
-			char u[16];
-			if ((exp->e_flags & flag_mask & NFSEXP_FSID) == 0 &&
-			    uuid_by_path(path, 0, 16, u)) {
-				qword_add(&bp, &blen, "uuid");
-				qword_addhex(&bp, &blen, u, 16);
-			}
-		} else {
-			char u[16];
-			get_uuid(exp->e_uuid, 16, u);
+		write_secinfo(&bp, &blen, exp, ea.sec_mask, ea.sec_extra);
+		if (ea.have_uuid) {
 			qword_add(&bp, &blen, "uuid");
-			qword_addhex(&bp, &blen, u, 16);
+			qword_addhex(&bp, &blen, ea.uuid, 16);
 		}
 		write_xprtsec(&bp, &blen, exp);
 		xlog(D_AUTH, "granted access to %s for %s",

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH nfs-utils 2/5] mountd: handle unmountable paths and junctions in the netlink downcall
  2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 1/5] mountd: factor out the per-path export attribute computation Jeff Layton
@ 2026-09-09 12:56 ` Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 3/5] mountd: answer requests the kernel rejects on " Jeff Layton
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

Two things nfsd_export() does that cache_nl_process_export() did not:

 - If is_mountpoint() fails with an error that isn't a plain lookup
   failure, we can't tell whether the export is available.  Defer the
   request instead of answering "not exported".

 - When no export matches, check for a junction before denying, so
   referrals still work.  Only when the client resolved, as nfsd_export()
   does: client_check() dereferences the addrinfo for wildcard and
   netgroup clients.

Split lookup_nonexport() so both downcalls share the junction lookup, and
factor the per-request work into nl_add_export_req() so the deferred path
can reuse it.  Deferred requests go on their own list, deduped on
(client, path) and retried from cache_process() every RETRY_SEC.  The
kernel keeps the upcall pending in the meantime.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 support/export/cache.c | 304 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 238 insertions(+), 66 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index e8b13409ad7a..ed90a29a0ec7 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -784,6 +784,18 @@ struct delayed {
 	struct delayed *next;
 } *delayed;
 
+/* Fold one retry queue head's deadline into the running minimum */
+static time_t retry_delay(time_t *last_attempt, time_t now, time_t delay)
+{
+	time_t d;
+
+	if (*last_attempt > now)
+		/* Clock updated - retry immediately */
+		*last_attempt = now - RETRY_SEC;
+	d = *last_attempt + RETRY_SEC - now;
+	return d < delay ? d : delay;
+}
+
 static int nfsd_handle_fh(int f, char *bp, int blen)
 {
 	/* request are:
@@ -1162,8 +1174,18 @@ static int export_attrs_build(struct export_attrs *ea, char *path,
  * NFSD_CMD_SVC_EXPORT_SET_REQS.
  */
 static nfs_export *lookup_export(char *dom, char *path, struct addrinfo *ai);
+static struct exportent *lookup_nonexport_ent(char *dom, char *path,
+					      struct addrinfo *ai);
 static struct nl_msg *cache_nl_new_msg(int family, int cmd, int flags);
 
+static void free_junction(struct exportent *eep)
+{
+	if (!eep)
+		return;
+	exportent_release(eep);
+	free(eep);
+}
+
 static struct nl_sock *nfsd_nl_notify_sock;	/* multicast notifications */
 static struct nl_sock *nfsd_nl_cmd_sock;	/* GET_REQS / SET_REQS commands */
 static int nfsd_nl_family;
@@ -1632,6 +1654,187 @@ static int cache_nl_set_reqs(struct nl_sock *sock, struct nl_msg *msg)
 	return ret;
 }
 
+static bool nl_msg_has_reqs(struct nl_msg *msg)
+{
+	return genlmsg_attrlen(nlmsg_data(nlmsg_hdr(msg)), 0) > 0;
+}
+
+enum export_result {
+	EXPORT_ANSWERED,
+	EXPORT_RETRY,		/* not resolvable yet, ask again later */
+	EXPORT_NOMEM,		/* *msgp is gone, caller must give up */
+};
+
+/*
+ * Resolve one svc_export request and append the answer to *msgp, sending
+ * and replacing the message if it fills up.
+ */
+static enum export_result nl_add_export_req(struct nl_msg **msgp, char *dom,
+					    char *path)
+{
+	struct addrinfo *ai = NULL;
+	nfs_export *found = NULL;
+	struct exportent *epp = NULL;
+	struct exportent *junction = NULL;
+	enum export_result res = EXPORT_ANSWERED;
+	int ttl = 0;
+
+	if (is_ipaddr_client(dom)) {
+		ai = lookup_client_addr(dom);
+		if (!ai)
+			xlog(D_AUTH, "%s: failed to resolve client %s",
+			     __func__, dom);
+	}
+
+	if (ai || !is_ipaddr_client(dom)) {
+		found = lookup_export(dom, path, ai);
+		if (!found) {
+			junction = lookup_nonexport_ent(dom, path, ai);
+			epp = junction;
+		}
+	}
+
+	if (found) {
+		char *mp = found->m_export.e_mountpoint;
+
+		if (mp && !*mp)
+			mp = found->m_export.e_path;
+		errno = 0;
+		if (mp && !is_mountpoint(mp)) {
+			/*
+			 * A strange error means we can't tell whether it is a
+			 * mountpoint.  Retry later rather than answer wrongly.
+			 */
+			if (errno != 0 && !path_lookup_error(errno)) {
+				res = EXPORT_RETRY;
+				goto out;
+			}
+			/* Exportpoint is not mounted, so tell kernel it
+			 * is not available.  This will cause it not to
+			 * appear in the V4 Pseudo-root, so a "mount" of
+			 * this path will fail, just like with V3.
+			 */
+			xlog(L_WARNING,
+			     "Cannot export path '%s': not a mountpoint", path);
+			ttl = 60;
+		} else {
+			epp = &found->m_export;
+		}
+	}
+
+	if (nfsd_nl_add_export(*msgp, dom, path, epp, ttl) < 0) {
+		cache_nl_set_reqs(nfsd_nl_cmd_sock, *msgp);
+		nlmsg_free(*msgp);
+		*msgp = cache_nl_new_msg(nfsd_nl_family,
+					 NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
+		if (!*msgp) {
+			res = EXPORT_NOMEM;
+			goto out;
+		}
+		if (nfsd_nl_add_export(*msgp, dom, path, epp, ttl) < 0)
+			xlog(L_WARNING, "%s: skipping oversized entry for %s",
+			     __func__, path);
+	}
+out:
+	free_junction(junction);
+	nfs_freeaddrinfo(ai);
+	return res;
+}
+
+/*
+ * is_mountpoint() can fail with a strange error - the ETIMEDOUT a re-exported
+ * "softerr" NFS mount can give, say - leaving us unable to say whether the
+ * path is exportable.  Set the request aside and try again later.
+ */
+struct delayed_export {
+	char			*client;
+	char			*path;
+	time_t			last_attempt;
+	struct delayed_export	*next;
+};
+
+static struct delayed_export *delayed_export;
+
+static void delayed_export_enqueue(struct delayed_export *d)
+{
+	struct delayed_export **dp = &delayed_export;
+
+	d->last_attempt = time(NULL);
+	d->next = NULL;
+	while (*dp)
+		dp = &(*dp)->next;
+	*dp = d;
+}
+
+static void delayed_export_free(struct delayed_export *d)
+{
+	free(d->client);
+	free(d->path);
+	free(d);
+}
+
+static void nl_delay_export(char *dom, char *path)
+{
+	struct delayed_export *d;
+
+	for (d = delayed_export; d; d = d->next)
+		if (!strcmp(d->client, dom) && !strcmp(d->path, path))
+			return;
+
+	d = calloc(1, sizeof(*d));
+	if (!d)
+		return;
+
+	d->client = strdup(dom);
+	d->path = strdup(path);
+	if (!d->client || !d->path) {
+		delayed_export_free(d);
+		return;
+	}
+
+	delayed_export_enqueue(d);
+}
+
+/*
+ * Retry the oldest deferred request if it is due.  Entries are queued in
+ * time order, so only the head can be ready.
+ */
+static void nl_retry_export(void)
+{
+	struct delayed_export *d = delayed_export;
+	struct nl_msg *msg;
+
+	if (!d || d->last_attempt + RETRY_SEC > time(NULL))
+		return;
+
+	delayed_export = d->next;
+	d->next = NULL;
+
+	auth_reload();
+
+	msg = cache_nl_new_msg(nfsd_nl_family,
+			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
+	if (!msg) {
+		delayed_export_enqueue(d);
+		return;
+	}
+
+	switch (nl_add_export_req(&msg, d->client, d->path)) {
+	case EXPORT_ANSWERED:
+		if (nl_msg_has_reqs(msg))
+			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
+		delayed_export_free(d);
+		break;
+	case EXPORT_RETRY:
+		delayed_export_enqueue(d);
+		break;
+	case EXPORT_NOMEM:
+		delayed_export_enqueue(d);
+		return;			/* msg is already gone */
+	}
+	nlmsg_free(msg);
+}
+
 static void cache_nl_process_export(void)
 {
 	struct export_req *reqs = NULL;
@@ -1657,57 +1860,19 @@ static void cache_nl_process_export(void)
 		goto out_free;
 
 	for (i = 0; i < nreqs; i++) {
-		char *dom = reqs[i].client;
-		char *path = reqs[i].path;
-		struct addrinfo *ai = NULL;
-		nfs_export *found = NULL;
-		struct exportent *epp = NULL;
-		int ttl = 0;
-
-		if (is_ipaddr_client(dom)) {
-			ai = lookup_client_addr(dom);
-			if (!ai)
-				xlog(D_AUTH, "cache_nl_process_export: "
-				     "failed to resolve client %s", dom);
-		}
-
-		if (ai || !is_ipaddr_client(dom))
-			found = lookup_export(dom, path, ai);
-
-		if (found) {
-			char *mp = found->m_export.e_mountpoint;
-
-			if (mp && !*mp)
-				mp = found->m_export.e_path;
-			if (mp && !is_mountpoint(mp)) {
-				xlog(L_WARNING,
-				     "Cannot export path '%s': not a mountpoint",
-				     path);
-				ttl = 60;
-			} else {
-				epp = &found->m_export;
-			}
-		}
-
-		if (nfsd_nl_add_export(msg, dom, path, epp, ttl) < 0) {
-			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
-			nlmsg_free(msg);
-			msg = cache_nl_new_msg(nfsd_nl_family,
-					       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
-			if (!msg) {
-				nfs_freeaddrinfo(ai);
-				goto out_free;
-			}
-			if (nfsd_nl_add_export(msg, dom, path,
-					       epp, ttl) < 0)
-				xlog(L_WARNING, "%s: skipping oversized "
-				     "entry for %s", __func__, path);
+		switch (nl_add_export_req(&msg, reqs[i].client, reqs[i].path)) {
+		case EXPORT_ANSWERED:
+			break;
+		case EXPORT_RETRY:
+			nl_delay_export(reqs[i].client, reqs[i].path);
+			break;
+		case EXPORT_NOMEM:
+			goto out_free;
 		}
-
-		nfs_freeaddrinfo(ai);
 	}
 
-	cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
+	if (nl_msg_has_reqs(msg))
+		cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
 	nlmsg_free(msg);
 
 out_free:
@@ -2971,29 +3136,32 @@ out:
 	return exp;
 }
 
-static void lookup_nonexport(int f, char *buf, int buflen, char *dom, char *path,
+static struct exportent *lookup_nonexport_ent(char *dom, char *path,
 		struct addrinfo *ai)
 {
-	struct exportent *eep;
-
-	eep = lookup_junction(dom, path, ai);
-	dump_to_cache(f, buf, buflen, dom, path, eep, 0);
-	if (eep == NULL)
-		return;
-	exportent_release(eep);
-	free(eep);
+	return lookup_junction(dom, path, ai);
 }
 
 #else	/* !HAVE_JUNCTION_SUPPORT */
 
-static void lookup_nonexport(int f, char *buf, int buflen, char *dom, char *path,
-		struct addrinfo *UNUSED(ai))
+static struct exportent *lookup_nonexport_ent(char *UNUSED(dom),
+		char *UNUSED(path), struct addrinfo *UNUSED(ai))
 {
-	dump_to_cache(f, buf, buflen, dom, path, NULL, 0);
+	return NULL;
 }
 
 #endif	/* !HAVE_JUNCTION_SUPPORT */
 
+static void lookup_nonexport(int f, char *buf, int buflen, char *dom, char *path,
+		struct addrinfo *ai)
+{
+	struct exportent *eep;
+
+	eep = lookup_nonexport_ent(dom, path, ai);
+	dump_to_cache(f, buf, buflen, dom, path, eep, 0);
+	free_junction(eep);
+}
+
 static void nfsd_export(int f)
 {
 	/* requests are:
@@ -3201,13 +3369,15 @@ int cache_process(fd_set *readfds)
 	cache_set_fds(readfds);
 	v4clients_set_fds(readfds);
 
-	if (delayed) {
+	if (delayed || delayed_export) {
 		time_t now = time(NULL);
-		time_t delay;
-		if (delayed->last_attempt > now)
-			/* Clock updated - retry immediately */
-			delayed->last_attempt = now - RETRY_SEC;
-		delay = delayed->last_attempt + RETRY_SEC - now;
+		time_t delay = RETRY_SEC;
+
+		if (delayed)
+			delay = retry_delay(&delayed->last_attempt, now, delay);
+		if (delayed_export)
+			delay = retry_delay(&delayed_export->last_attempt, now,
+					    delay);
 		if (delay < 0)
 			delay = 0;
 		tv.tv_sec = delay;
@@ -3225,6 +3395,8 @@ int cache_process(fd_set *readfds)
 		}
 	}
 
+	nl_retry_export();
+
 	switch (selret) {
 	case -1:
 		if (errno == EINTR || errno == ECONNREFUSED

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH nfs-utils 3/5] mountd: answer requests the kernel rejects on the netlink downcall
  2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 1/5] mountd: factor out the per-path export attribute computation Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 2/5] mountd: handle unmountable paths and junctions in the netlink downcall Jeff Layton
@ 2026-09-09 12:56 ` Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 4/5] mountd: don't leak the parent export's fsid onto crossmnt submounts Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 5/5] mountd: retry unresolvable fsid lookups on the netlink downcall Jeff Layton
  4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

The kernel refuses an svc_export it cannot build a filehandle for - a 9p
submount picked up by crossmnt, say - and it fails the whole SET_REQS
message when it does.  cache_nl_process_export() batches every pending
request into one message and never looks at the result, so:

 - every entry queued behind the bad one is dropped
 - nothing downgrades the bad path to a negative entry
 - the request stays pending and the client hangs on the lookup

Pending requests then pile up on each notification:

  cache_nl_process_export: 5 pending export requests
  cache_nl_set_reqs: SET_REQS failed: -7

nfsd_export() has no such hole: a rejected dump_to_cache() write returns
-1 and it answers negative instead.

Keep the batch for the fast path, but resubmit it one entry at a time
when the kernel rejects it, and answer negative for whichever entries it
still refuses.  nfsd_nl_svc_export_set_reqs_doit() commits each entry as
it parses it and stops at the first failure, so the resubmit re-sends
some entries the kernel already took; that is harmless, as the update is
idempotent, and it is the only way to find the one that failed.
nl_add_export_req() no longer flushes the message itself; the caller
owns the batch so it knows what to resubmit.

Only fall back to a negative entry when the kernel actually answered.
libnl folds its own errors into the same NLE_* space as the kernel's, so
cache_nl_set_reqs() now reports the kernel's errno separately: a broken
socket must not cache "not exported" for default_ttl.  A negative entry
the kernel already refused - the path or the client's auth_domain went
away - is not resent as another negative either.

An entry too large for a message of its own is a third case.  mountd
never sent it, so the kernel never refused it.  Log the size failure and
answer negative, rather than blame the filesystem for it.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 support/export/cache.c | 249 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 177 insertions(+), 72 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index ed90a29a0ec7..d118834da29a 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -1623,18 +1623,55 @@ static struct nl_msg *cache_nl_new_msg(int family, int cmd, int flags)
 	return msg;
 }
 
-static int cache_nl_set_reqs(struct nl_sock *sock, struct nl_msg *msg)
+/* State for one SET_REQS round trip */
+struct set_reqs_status {
+	int	done;
+	int	kern_err;	/* errno the kernel answered with */
+};
+
+static int nl_set_reqs_finish_cb(struct nl_msg *UNUSED(msg), void *arg)
+{
+	struct set_reqs_status *st = arg;
+
+	st->done = 1;
+	return NL_STOP;
+}
+
+static int nl_set_reqs_error_cb(struct sockaddr_nl *UNUSED(nla),
+				struct nlmsgerr *nlerr, void *arg)
 {
+	struct set_reqs_status *st = arg;
+
+	st->done = 1;
+	st->kern_err = nlerr->error;
+	return NL_STOP;
+}
+
+/*
+ * Send @msg and wait for the kernel to ack it.  Returns 0 on success.
+ *
+ * libnl folds the kernel's errno into its own NLE_* space, and a local
+ * failure lands in the same space, so the return value cannot say whether
+ * the kernel looked at the message at all.  When @kern_errp is given it is
+ * set to the negative errno the kernel replied with, or left at 0 if it
+ * never got that far.
+ */
+static int cache_nl_set_reqs(struct nl_sock *sock, struct nl_msg *msg,
+			     int *kern_errp)
+{
+	struct set_reqs_status st = {};
 	struct nl_cb *cb;
-	int done = 0;
 	int ret;
 
+	if (kern_errp)
+		*kern_errp = 0;
+
 	cb = nl_cb_alloc(NL_CB_DEFAULT);
 	if (!cb)
 		return -ENOMEM;
 
-	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, nl_finish_cb, &done);
-	nl_cb_err(cb, NL_CB_CUSTOM, nl_error_cb, &done);
+	nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, nl_set_reqs_finish_cb, &st);
+	nl_cb_err(cb, NL_CB_CUSTOM, nl_set_reqs_error_cb, &st);
 
 	ret = nl_send_auto(sock, msg);
 	if (ret < 0) {
@@ -1642,13 +1679,15 @@ static int cache_nl_set_reqs(struct nl_sock *sock, struct nl_msg *msg)
 		return ret;
 	}
 
-	while (!done) {
+	while (!st.done) {
 		ret = nl_recvmsgs(sock, cb);
 		if (ret < 0)
 			break;
 	}
 
 	nl_cb_put(cb);
+	if (kern_errp)
+		*kern_errp = st.kern_err;
 	if (ret < 0)
 		xlog(L_WARNING, "%s: SET_REQS failed: %d", __func__, ret);
 	return ret;
@@ -1660,23 +1699,21 @@ static bool nl_msg_has_reqs(struct nl_msg *msg)
 }
 
 enum export_result {
-	EXPORT_ANSWERED,
+	EXPORT_ANSWERED,	/* positive entry added */
+	EXPORT_DENIED,		/* negative entry added */
 	EXPORT_RETRY,		/* not resolvable yet, ask again later */
-	EXPORT_NOMEM,		/* *msgp is gone, caller must give up */
+	EXPORT_FULL,		/* did not fit, flush the message and re-add */
 };
 
-/*
- * Resolve one svc_export request and append the answer to *msgp, sending
- * and replacing the message if it fills up.
- */
-static enum export_result nl_add_export_req(struct nl_msg **msgp, char *dom,
+/* Resolve one svc_export request and append the answer to @msg */
+static enum export_result nl_add_export_req(struct nl_msg *msg, char *dom,
 					    char *path)
 {
 	struct addrinfo *ai = NULL;
 	nfs_export *found = NULL;
 	struct exportent *epp = NULL;
 	struct exportent *junction = NULL;
-	enum export_result res = EXPORT_ANSWERED;
+	enum export_result res;
 	int ttl = 0;
 
 	if (is_ipaddr_client(dom)) {
@@ -1722,25 +1759,90 @@ static enum export_result nl_add_export_req(struct nl_msg **msgp, char *dom,
 		}
 	}
 
-	if (nfsd_nl_add_export(*msgp, dom, path, epp, ttl) < 0) {
-		cache_nl_set_reqs(nfsd_nl_cmd_sock, *msgp);
-		nlmsg_free(*msgp);
-		*msgp = cache_nl_new_msg(nfsd_nl_family,
-					 NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
-		if (!*msgp) {
-			res = EXPORT_NOMEM;
-			goto out;
-		}
-		if (nfsd_nl_add_export(*msgp, dom, path, epp, ttl) < 0)
-			xlog(L_WARNING, "%s: skipping oversized entry for %s",
-			     __func__, path);
-	}
+	if (nfsd_nl_add_export(msg, dom, path, epp, ttl) < 0)
+		res = EXPORT_FULL;
+	else
+		res = epp ? EXPORT_ANSWERED : EXPORT_DENIED;
 out:
 	free_junction(junction);
 	nfs_freeaddrinfo(ai);
 	return res;
 }
 
+static void nl_export_negative(char *dom, char *path)
+{
+	struct nl_msg *msg;
+
+	msg = cache_nl_new_msg(nfsd_nl_family,
+			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
+	if (!msg)
+		return;
+	if (nfsd_nl_add_export(msg, dom, path, NULL, 0) == 0)
+		cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
+	nlmsg_free(msg);
+}
+
+/*
+ * Answer one request in a message of its own.  The kernel rejects an export
+ * it cannot build a filehandle for - a 9p or other filesystem with no export
+ * ops, say - so fall back to a negative entry rather than leaving the request
+ * pending, which is what dump_to_cache() does when the channel write fails.
+ */
+static enum export_result nl_export_one(char *dom, char *path)
+{
+	enum export_result res;
+	struct nl_msg *msg;
+	int kern_err = 0;
+	bool sent;
+
+	msg = cache_nl_new_msg(nfsd_nl_family,
+			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
+	if (!msg)
+		return EXPORT_RETRY;
+
+	res = nl_add_export_req(msg, dom, path);
+	if (res == EXPORT_RETRY) {
+		nlmsg_free(msg);
+		return res;
+	}
+
+	/*
+	 * An entry that does not fit a message of its own can never be sent,
+	 * and this is not the kernel refusing it.  Answer negative rather
+	 * than leave the client hung on a request we cannot satisfy.
+	 */
+	if (res == EXPORT_FULL) {
+		nlmsg_free(msg);
+		xlog(L_WARNING, "%s: entry for %s is too large to send",
+		     __func__, path);
+		nl_export_negative(dom, path);
+		return EXPORT_ANSWERED;
+	}
+
+	sent = cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, &kern_err) == 0;
+	nlmsg_free(msg);
+
+	if (sent)
+		return EXPORT_ANSWERED;
+
+	/*
+	 * The kernel never answered - a broken socket, or a message we could
+	 * not send - so we cannot tell whether the export is usable.  Retry
+	 * rather than cache a negative entry for default_ttl.
+	 */
+	if (!kern_err)
+		return EXPORT_RETRY;
+
+	/* It refused a negative entry; a second one will fare no better */
+	if (res == EXPORT_DENIED)
+		return EXPORT_ANSWERED;
+
+	xlog(L_WARNING, "Cannot export %s, possibly unsupported filesystem"
+	     " or fsid= required", path);
+	nl_export_negative(dom, path);
+	return EXPORT_ANSWERED;
+}
+
 /*
  * is_mountpoint() can fail with a strange error - the ETIMEDOUT a re-exported
  * "softerr" NFS mount can give, say - leaving us unable to say whether the
@@ -1802,7 +1904,6 @@ static void nl_delay_export(char *dom, char *path)
 static void nl_retry_export(void)
 {
 	struct delayed_export *d = delayed_export;
-	struct nl_msg *msg;
 
 	if (!d || d->last_attempt + RETRY_SEC > time(NULL))
 		return;
@@ -1812,35 +1913,26 @@ static void nl_retry_export(void)
 
 	auth_reload();
 
-	msg = cache_nl_new_msg(nfsd_nl_family,
-			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
-	if (!msg) {
+	if (nl_export_one(d->client, d->path) == EXPORT_RETRY)
 		delayed_export_enqueue(d);
-		return;
-	}
-
-	switch (nl_add_export_req(&msg, d->client, d->path)) {
-	case EXPORT_ANSWERED:
-		if (nl_msg_has_reqs(msg))
-			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
+	else
 		delayed_export_free(d);
-		break;
-	case EXPORT_RETRY:
-		delayed_export_enqueue(d);
-		break;
-	case EXPORT_NOMEM:
-		delayed_export_enqueue(d);
-		return;			/* msg is already gone */
-	}
-	nlmsg_free(msg);
+}
+
+static void nl_export_singly(struct export_req *reqs, int start, int end)
+{
+	int i;
+
+	for (i = start; i < end; i++)
+		if (nl_export_one(reqs[i].client, reqs[i].path) == EXPORT_RETRY)
+			nl_delay_export(reqs[i].client, reqs[i].path);
 }
 
 static void cache_nl_process_export(void)
 {
 	struct export_req *reqs = NULL;
 	int nreqs = 0;
-	struct nl_msg *msg;
-	int i;
+	int i = 0;
 
 	/* Fetch all pending requests from the kernel */
 	if (cache_nl_get_export_reqs(&reqs, &nreqs)) {
@@ -1853,29 +1945,42 @@ static void cache_nl_process_export(void)
 
 	xlog(D_CALL, "cache_nl_process_export: %d pending export requests", nreqs);
 
-	/* Build the SET_REQS response */
-	msg = cache_nl_new_msg(nfsd_nl_family,
-			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
-	if (!msg)
-		goto out_free;
+	while (i < nreqs) {
+		int start = i;
+		struct nl_msg *msg;
 
-	for (i = 0; i < nreqs; i++) {
-		switch (nl_add_export_req(&msg, reqs[i].client, reqs[i].path)) {
-		case EXPORT_ANSWERED:
-			break;
-		case EXPORT_RETRY:
-			nl_delay_export(reqs[i].client, reqs[i].path);
+		msg = cache_nl_new_msg(nfsd_nl_family,
+				       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
+		if (!msg)
 			break;
-		case EXPORT_NOMEM:
-			goto out_free;
+
+		for (; i < nreqs; i++) {
+			enum export_result res;
+
+			res = nl_add_export_req(msg, reqs[i].client,
+						reqs[i].path);
+			if (res == EXPORT_FULL)
+				break;
+			if (res == EXPORT_RETRY)
+				nl_delay_export(reqs[i].client, reqs[i].path);
 		}
-	}
 
-	if (nl_msg_has_reqs(msg))
-		cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
-	nlmsg_free(msg);
+		/*
+		 * One bad entry fails the whole message, so resubmit the
+		 * batch singly to find out which and answer the rest.
+		 */
+		if (nl_msg_has_reqs(msg) &&
+		    cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL) < 0)
+			nl_export_singly(reqs, start, i);
+		nlmsg_free(msg);
+
+		/* First entry did not fit an empty message: answer it alone */
+		if (i == start) {
+			nl_export_singly(reqs, i, i + 1);
+			i++;
+		}
+	}
 
-out_free:
 	for (i = 0; i < nreqs; i++) {
 		free(reqs[i].client);
 		free(reqs[i].path);
@@ -2145,7 +2250,7 @@ static void cache_nl_process_expkey(void)
 do_add_expkey:
 		if (nfsd_nl_add_expkey(msg, dom, fsidtype, fsid,
 				       fsidlen, found_path) < 0) {
-			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
+			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
 			nlmsg_free(msg);
 			msg = cache_nl_new_msg(nfsd_nl_family,
 					       NFSD_CMD_EXPKEY_SET_REQS, 0);
@@ -2166,7 +2271,7 @@ do_add_expkey:
 		nfs_freeaddrinfo(ai);
 	}
 
-	cache_nl_set_reqs(nfsd_nl_cmd_sock, msg);
+	cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
 	nlmsg_free(msg);
 
 out_free:
@@ -2456,7 +2561,7 @@ static void cache_nl_process_ip_map(void)
 		}
 
 		if (nl_add_ip_map(msg, class, ipaddr, domain) < 0) {
-			cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg);
+			cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg, NULL);
 			nlmsg_free(msg);
 			msg = cache_nl_new_msg(sunrpc_nl_family,
 					       SUNRPC_CMD_IP_MAP_SET_REQS, 0);
@@ -2486,7 +2591,7 @@ static void cache_nl_process_ip_map(void)
 		nfs_freeaddrinfo(tmp);
 	}
 
-	cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg);
+	cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg, NULL);
 	nlmsg_free(msg);
 
 out_free:
@@ -2704,7 +2809,7 @@ static void cache_nl_process_unix_gid(void)
 
 		if (ret < 0) {
 			/* Flush current message and retry with a fresh one */
-			cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg);
+			cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg, NULL);
 			nlmsg_free(msg);
 			msg = cache_nl_new_msg(sunrpc_nl_family,
 					       SUNRPC_CMD_UNIX_GID_SET_REQS, 0);
@@ -2721,7 +2826,7 @@ static void cache_nl_process_unix_gid(void)
 		}
 	}
 
-	cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg);
+	cache_nl_set_reqs(sunrpc_nl_cmd_sock, msg, NULL);
 	nlmsg_free(msg);
 
 out_free:

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH nfs-utils 4/5] mountd: don't leak the parent export's fsid onto crossmnt submounts
  2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
                   ` (2 preceding siblings ...)
  2026-09-09 12:56 ` [PATCH nfs-utils 3/5] mountd: answer requests the kernel rejects on " Jeff Layton
@ 2026-09-09 12:56 ` Jeff Layton
  2026-09-09 12:56 ` [PATCH nfs-utils 5/5] mountd: retry unresolvable fsid lookups on the netlink downcall Jeff Layton
  4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

nfsd_nl_add_export() sent e_flags and e_fsid straight from the parent
exportent, unlike dump_to_cache() which strips NFSEXP_FSID (and picks a
path-derived uuid) when the upcall path is a submount below the exported
one.

v4root forces "/" to fsid=0, so exporting "/" with crossmnt handed every
submount fsid=0 as well.  Both exports then encode the same fsid, the
submount's filehandles decode back to "/", and the client sees

  NFS: server X error: fileid changed
  fsid 0:150: expected fileid 0x2, got 0x100

followed by ESTALE.  Re-export via fsidnum was missing for the same
reason, so a re-exported submount got the parent's fsid too.

Use export_attrs_build() for the flags, fsid and uuid, mask the per-
flavor secinfo flags to match (the kernel rejects the entry otherwise),
and fall back to a negative entry when the export cannot be resolved.
While here, honour the export's own e_ttl on positive entries.

Two consequences worth noting.  A crossmnt submount under an fsid=
parent now gets its own filehandles, so clients holding the old
(aliased) ones see ESTALE once - that aliasing was the bug.  And a
crossmnt submount on a filesystem with no blkid uuid and no statfs fsid
(tmpfs, say) now has neither fsid= nor uuid, so check_export() refuses
it; the preceding patch turns that into a negative entry instead of a
failed batch, so this one depends on it.

Reported-by: Mantas Mikulėnas <grawity@gmail.com>
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 support/export/cache.c | 53 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index d118834da29a..1f81a67e35cf 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -1500,7 +1500,8 @@ static int nfsd_nl_add_fsloc(struct nl_msg *msg, struct exportent *ep)
 	return 0;
 }
 
-static int nfsd_nl_add_secinfo(struct nl_msg *msg, struct exportent *ep)
+static int nfsd_nl_add_secinfo(struct nl_msg *msg, struct exportent *ep,
+			       struct export_attrs *ea)
 {
 	struct sec_entry *p;
 
@@ -1520,7 +1521,7 @@ static int nfsd_nl_add_secinfo(struct nl_msg *msg, struct exportent *ep)
 		if (nla_put_u32(msg, NFSD_A_AUTH_FLAVOR_PSEUDOFLAVOR,
 				p->flav->fnum) < 0 ||
 		    nla_put_u32(msg, NFSD_A_AUTH_FLAVOR_FLAGS,
-				p->flags) < 0)
+				(p->flags | ea->sec_extra) & ea->sec_mask) < 0)
 			return -1;
 		nla_nest_end(msg, sec);
 	}
@@ -1545,15 +1546,26 @@ static int nfsd_nl_add_xprtsec(struct nl_msg *msg, struct exportent *ep)
 	return 0;
 }
 
+/*
+ * Add one svc_export response.  @ea must be the attributes computed by
+ * export_attrs_build() for (@path, @exp), and is ignored when @exp is NULL.
+ * The only failure mode is a full message, so the caller can retry with a
+ * fresh one.
+ */
 static int nfsd_nl_add_export(struct nl_msg *msg, char *domain, char *path,
-			 struct exportent *exp, int ttl)
+			 struct exportent *exp, struct export_attrs *ea,
+			 int ttl)
 {
 	struct nlattr *nest;
 	time_t now = time(0);
-	char u[16];
+	uint64_t expiry;
 
+	/* A positive entry carries the export's own ttl */
+	if (exp)
+		ttl = (int)exp->e_ttl;
 	if (ttl <= 1)
 		ttl = default_ttl;
+	expiry = now + ttl;
 
 	nest = nla_nest_start(msg, NFSD_A_SVC_EXPORT_REQS_REQUESTS);
 	if (!nest)
@@ -1561,7 +1573,7 @@ static int nfsd_nl_add_export(struct nl_msg *msg, char *domain, char *path,
 
 	if (nla_put_string(msg, NFSD_A_SVC_EXPORT_CLIENT, domain) < 0 ||
 	    nla_put_string(msg, NFSD_A_SVC_EXPORT_PATH, path) < 0 ||
-	    nla_put_u64(msg, NFSD_A_SVC_EXPORT_EXPIRY, now + ttl) < 0)
+	    nla_put_u64(msg, NFSD_A_SVC_EXPORT_EXPIRY, expiry) < 0)
 		goto nla_failure;
 
 	if (!exp) {
@@ -1573,26 +1585,19 @@ static int nfsd_nl_add_export(struct nl_msg *msg, char *domain, char *path,
 		    nla_put_u32(msg, NFSD_A_SVC_EXPORT_ANON_GID,
 				exp->e_anongid) < 0 ||
 		    nla_put_u32(msg, NFSD_A_SVC_EXPORT_FLAGS,
-				exp->e_flags) < 0 ||
+				ea->flags) < 0 ||
 		    nla_put_s32(msg, NFSD_A_SVC_EXPORT_FSID,
-				exp->e_fsid) < 0)
+				ea->fsidnum) < 0)
 			goto nla_failure;
 
 		if (nfsd_nl_add_fsloc(msg, exp))
 			goto nla_failure;
 
-		if (exp->e_uuid) {
-			get_uuid(exp->e_uuid, 16, u);
-			if (nla_put(msg, NFSD_A_SVC_EXPORT_UUID,
-				    16, u) < 0)
-				goto nla_failure;
-		} else if (uuid_by_path(path, 0, 16, u)) {
-			if (nla_put(msg, NFSD_A_SVC_EXPORT_UUID,
-				    16, u) < 0)
-				goto nla_failure;
-		}
+		if (ea->have_uuid &&
+		    nla_put(msg, NFSD_A_SVC_EXPORT_UUID, 16, ea->uuid) < 0)
+			goto nla_failure;
 
-		if (nfsd_nl_add_secinfo(msg, exp))
+		if (nfsd_nl_add_secinfo(msg, exp, ea))
 			goto nla_failure;
 
 		if (nfsd_nl_add_xprtsec(msg, exp))
@@ -1713,6 +1718,7 @@ static enum export_result nl_add_export_req(struct nl_msg *msg, char *dom,
 	nfs_export *found = NULL;
 	struct exportent *epp = NULL;
 	struct exportent *junction = NULL;
+	struct export_attrs ea = {};
 	enum export_result res;
 	int ttl = 0;
 
@@ -1759,7 +1765,14 @@ static enum export_result nl_add_export_req(struct nl_msg *msg, char *dom,
 		}
 	}
 
-	if (nfsd_nl_add_export(msg, dom, path, epp, ttl) < 0)
+	if (epp && export_attrs_build(&ea, path, epp) < 0) {
+		xlog(L_WARNING, "Cannot export %s, possibly unsupported"
+		     " filesystem or fsid= required", path);
+		epp = NULL;
+		ttl = 0;
+	}
+
+	if (nfsd_nl_add_export(msg, dom, path, epp, &ea, ttl) < 0)
 		res = EXPORT_FULL;
 	else
 		res = epp ? EXPORT_ANSWERED : EXPORT_DENIED;
@@ -1777,7 +1790,7 @@ static void nl_export_negative(char *dom, char *path)
 			       NFSD_CMD_SVC_EXPORT_SET_REQS, 0);
 	if (!msg)
 		return;
-	if (nfsd_nl_add_export(msg, dom, path, NULL, 0) == 0)
+	if (nfsd_nl_add_export(msg, dom, path, NULL, NULL, 0) == 0)
 		cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
 	nlmsg_free(msg);
 }

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH nfs-utils 5/5] mountd: retry unresolvable fsid lookups on the netlink downcall
  2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
                   ` (3 preceding siblings ...)
  2026-09-09 12:56 ` [PATCH nfs-utils 4/5] mountd: don't leak the parent export's fsid onto crossmnt submounts Jeff Layton
@ 2026-09-09 12:56 ` Jeff Layton
  4 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2026-09-09 12:56 UTC (permalink / raw)
  To: Steve Dickson, Mantas Mikulėnas; +Cc: Chuck Lever, linux-nfs, Jeff Layton

nfsd_fh() defers a request instead of an answer in two cases:

 - the fsid names a device that is not present
 - the export has a "mountpoint" that is not mounted

The filesystem can appear soon, and an answer now gives a spurious
ESTALE.  cache_nl_process_expkey() answered negative in both cases, so
a not-yet-mounted autofs submount failed immediately.

Move the fsid lookup out of nfsd_handle_fh() into lookup_fsid().  Call
it from both downcalls.  The netlink path then also gets:

 - dev_missing accounting for unmatchable and unmounted exports
 - reexpdb_uncover_subvolume() for re-exported fsidnums
 - the V4ROOT tie-break and duplicate-filehandle warning
 - the fsidtype range check

Deferred netlink requests use their own list.  cache_process() retries
them on the same RETRY_SEC cadence as the pipefs ones.  The kernel keeps
the upcall pending until then.

expkey has the same batching hole that svc_export had.
nfsd_nl_expkey_set_reqs_doit() stops at the first entry that it refuses.
It refuses an entry when kern_path() on the path fails.  It also refuses
an entry when auth_domain_find() on the client fails.  The kernel never
processed the entries behind the refused one.  Those clients hung.
Resubmit a rejected batch one entry at a time, as the svc_export path
now does.

A refused entry gets no negative fallback.  The kernel builds a negative
expkey entry from auth_domain_find() alone, so a negative answer would
replace a path that kern_path() refused.  But nfsd_nl_add_expkey() sets
an expiry of 0x7fffffff.  A path that disappeared for a moment would
become a denial until the next "exportfs -f".  Log the entry and leave
it pending for the next upcall, which is what pipefs does when the
channel write fails.

A libnl or socket failure is a different case.  The kernel never saw the
message, so the request is still unanswered.  cache_nl_set_reqs()
reports the kernel's errno separately, and nl_expkey_one() returns such
a request to the retry list.

Reported-by: Mantas Mikulėnas <grawity@gmail.com>
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 support/export/cache.c | 415 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 280 insertions(+), 135 deletions(-)

diff --git a/support/export/cache.c b/support/export/cache.c
index 1f81a67e35cf..94d253565ab5 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -796,17 +796,19 @@ static time_t retry_delay(time_t *last_attempt, time_t now, time_t delay)
 	return d < delay ? d : delay;
 }
 
-static int nfsd_handle_fh(int f, char *bp, int blen)
+enum fsid_lookup {
+	FSID_LOOKUP_ANSWER,	/* definitive; *pathp NULL means deny access */
+	FSID_LOOKUP_RETRY,	/* not resolvable yet, ask again later */
+	FSID_LOOKUP_IGNORE,	/* unusable request, no reply possible */
+};
+
+/*
+ * Find the export path that @fsid refers to for client @dom.  On
+ * FSID_LOOKUP_ANSWER the caller owns *pathp.
+ */
+static enum fsid_lookup lookup_fsid(char *dom, int fsidtype, int fsidlen,
+				    char *fsid, char **pathp)
 {
-	/* request are:
-	 *  domain fsidtype fsid
-	 * interpret fsid, find export point and options, and write:
-	 *  domain fsidtype fsid expiry path
-	 */
-	char *dom;
-	int fsidtype;
-	int fsidlen;
-	char fsid[32];
 	struct parsed_fsid parsed;
 	struct exportent *found = NULL;
 	struct addrinfo *ai = NULL;
@@ -814,21 +816,13 @@ static int nfsd_handle_fh(int f, char *bp, int blen)
 	nfs_export *exp;
 	int i;
 	int dev_missing = 0;
-	char buf[RPC_CHAN_BUF_SIZE];
 	int did_uncover = 0;
-	int ret = 0;
+	enum fsid_lookup ret = FSID_LOOKUP_IGNORE;
+
+	*pathp = NULL;
 
-	dom = malloc(blen);
-	if (dom == NULL)
-		return ret;
-	if (qword_get(&bp, dom, blen) <= 0)
-		goto out;
-	if (qword_get_int(&bp, &fsidtype) != 0)
-		goto out;
 	if (fsidtype < 0 || fsidtype > 7)
 		goto out; /* unknown type */
-	if ((fsidlen = qword_get(&bp, fsid, 32)) <= 0)
-		goto out;
 	if (parse_fsid(fsidtype, fsidlen, fsid, &parsed))
 		goto out;
 
@@ -923,7 +917,7 @@ static int nfsd_handle_fh(int f, char *bp, int blen)
 		 * quiet rather than returning stale yet
 		 */
 		if (dev_missing) {
-			ret = 1;
+			ret = FSID_LOOKUP_RETRY;
 			goto out;
 		}
 	} else if (found->e_mountpoint &&
@@ -935,8 +929,55 @@ static int nfsd_handle_fh(int f, char *bp, int blen)
 		   xlog(L_WARNING, "%s not exported as %d not a mountpoint",
 		   found->e_path, found->e_mountpoint);
 		 */
+		ret = FSID_LOOKUP_RETRY;
+		goto out;
+	}
+
+	ret = FSID_LOOKUP_ANSWER;
+	*pathp = found_path;
+	found_path = NULL;
+out:
+	if (ret != FSID_LOOKUP_RETRY)
+		xlog(D_CALL, "%s: found %p path %s", __func__,
+		     found, found ? found->e_path : NULL);
+	free(found_path);
+	nfs_freeaddrinfo(ai);
+	return ret;
+}
+
+static int nfsd_handle_fh(int f, char *bp, int blen)
+{
+	/* request are:
+	 *  domain fsidtype fsid
+	 * interpret fsid, find export point and options, and write:
+	 *  domain fsidtype fsid expiry path
+	 */
+	char *dom;
+	int fsidtype;
+	int fsidlen;
+	char fsid[32];
+	char *found_path = NULL;
+	char buf[RPC_CHAN_BUF_SIZE];
+	int ret = 0;
+
+	dom = malloc(blen);
+	if (dom == NULL)
+		return ret;
+	if (qword_get(&bp, dom, blen) <= 0)
+		goto out;
+	if (qword_get_int(&bp, &fsidtype) != 0)
+		goto out;
+	if ((fsidlen = qword_get(&bp, fsid, 32)) <= 0)
+		goto out;
+
+	switch (lookup_fsid(dom, fsidtype, fsidlen, fsid, &found_path)) {
+	case FSID_LOOKUP_RETRY:
 		ret = 1;
 		goto out;
+	case FSID_LOOKUP_IGNORE:
+		goto out;
+	case FSID_LOOKUP_ANSWER:
+		break;
 	}
 
 	bp = buf; blen = sizeof(buf);
@@ -952,21 +993,16 @@ static int nfsd_handle_fh(int f, char *bp, int blen)
 	 * line.
 	 */
 	qword_addint(&bp, &blen, 0x7fffffff);
-	if (found)
+	if (found_path)
 		qword_add(&bp, &blen, found_path);
 	qword_addeol(&bp, &blen);
 	if (blen <= 0 || cache_write(f, buf, bp - buf) != bp - buf)
 		xlog(L_ERROR, "nfsd_fh: error writing reply");
-	if (!found)
+	if (!found_path)
 		xlog(D_AUTH, "denied access to %s", *dom == '$' ? dom+1 : dom);
 out:
-	if (found_path)
-		free(found_path);
-	nfs_freeaddrinfo(ai);
+	free(found_path);
 	free(dom);
-	if (!ret)
-		xlog(D_CALL, "nfsd_fh: found %p path %s",
-		     found, found ? found->e_path : NULL);
 	return ret;
 }
 
@@ -2161,12 +2197,192 @@ nla_failure:
 	return -1;
 }
 
+/*
+ * An fsid can name a filesystem that isn't mounted yet - an autofs
+ * mountpoint, or a re-exported NFS server that is slow to answer.  Set the
+ * request aside and try again later rather than declaring it stale, which
+ * is what nfsd_fh() does with the "delayed" list on pipefs.
+ */
+struct delayed_expkey {
+	char			*client;
+	char			*fsid;
+	int			fsidlen;
+	int			fsidtype;
+	time_t			last_attempt;
+	struct delayed_expkey	*next;
+};
+
+static struct delayed_expkey *delayed_expkey;
+
+static void delayed_expkey_enqueue(struct delayed_expkey *d)
+{
+	struct delayed_expkey **dp = &delayed_expkey;
+
+	d->last_attempt = time(NULL);
+	d->next = NULL;
+	while (*dp)
+		dp = &(*dp)->next;
+	*dp = d;
+}
+
+static void delayed_expkey_free(struct delayed_expkey *d)
+{
+	free(d->client);
+	free(d->fsid);
+	free(d);
+}
+
+static void nl_delay_expkey(struct expkey_req *req)
+{
+	struct delayed_expkey *d;
+
+	for (d = delayed_expkey; d; d = d->next)
+		if (d->fsidtype == req->fsidtype &&
+		    d->fsidlen == req->fsidlen &&
+		    !strcmp(d->client, req->client) &&
+		    !memcmp(d->fsid, req->fsid, req->fsidlen))
+			return;
+
+	d = calloc(1, sizeof(*d));
+	if (!d)
+		return;
+
+	d->client = strdup(req->client);
+	d->fsid = malloc(req->fsidlen);
+	if (!d->client || !d->fsid) {
+		delayed_expkey_free(d);
+		return;
+	}
+	memcpy(d->fsid, req->fsid, req->fsidlen);
+	d->fsidlen = req->fsidlen;
+	d->fsidtype = req->fsidtype;
+
+	delayed_expkey_enqueue(d);
+}
+
+enum expkey_result {
+	EXPKEY_ANSWERED,
+	EXPKEY_RETRY,		/* not resolvable yet, ask again later */
+	EXPKEY_FULL,		/* did not fit, flush the message and re-add */
+};
+
+/* Resolve one expkey request and append the answer to @msg */
+static enum expkey_result nl_add_expkey_req(struct nl_msg *msg,
+					    struct expkey_req *req)
+{
+	enum expkey_result res = EXPKEY_ANSWERED;
+	char *found_path = NULL;
+	char *dom = req->client;
+
+	switch (lookup_fsid(dom, req->fsidtype, req->fsidlen, req->fsid,
+			    &found_path)) {
+	case FSID_LOOKUP_RETRY:
+		return EXPKEY_RETRY;
+	case FSID_LOOKUP_IGNORE:	/* answer negative rather than hang */
+	case FSID_LOOKUP_ANSWER:
+		break;
+	}
+
+	if (nfsd_nl_add_expkey(msg, dom, req->fsidtype, req->fsid,
+			       req->fsidlen, found_path) < 0)
+		res = EXPKEY_FULL;
+	else if (!found_path)
+		xlog(D_AUTH, "denied access to %s", *dom == '$' ? dom + 1 : dom);
+
+	free(found_path);
+	return res;
+}
+
+/*
+ * Answer one request in a message of its own.  The kernel refuses an entry
+ * whose path or whose client's auth_domain has gone away, and fails the
+ * whole message when it does, so a rejected entry must not take the rest of
+ * a batch down with it.
+ */
+static enum expkey_result nl_expkey_one(struct expkey_req *req)
+{
+	enum expkey_result res;
+	struct nl_msg *msg;
+	int kern_err = 0;
+
+	msg = cache_nl_new_msg(nfsd_nl_family, NFSD_CMD_EXPKEY_SET_REQS, 0);
+	if (!msg)
+		return EXPKEY_RETRY;
+
+	res = nl_add_expkey_req(msg, req);
+	switch (res) {
+	case EXPKEY_ANSWERED:
+		if (cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, &kern_err) < 0) {
+			/*
+			 * The kernel never answered - a broken socket, or a
+			 * message we could not send - so the request is still
+			 * unanswered.  Ask again later rather than drop it.
+			 */
+			if (!kern_err) {
+				res = EXPKEY_RETRY;
+				break;
+			}
+			/*
+			 * Nothing to fall back on: a negative entry needs the
+			 * same auth_domain the kernel may have just failed to
+			 * find, and we cannot tell that apart from a path it
+			 * refused.  Leave the request pending for the next
+			 * upcall, as pipefs does when the channel write fails.
+			 */
+			xlog(L_WARNING, "%s: kernel rejected the fsid answer"
+			     " for %s: %d", __func__, req->client, kern_err);
+		}
+		break;
+	case EXPKEY_FULL:
+		xlog(L_WARNING, "%s: skipping oversized entry", __func__);
+		break;
+	case EXPKEY_RETRY:
+		break;
+	}
+	nlmsg_free(msg);
+	return res;
+}
+
+static void nl_expkey_singly(struct expkey_req *reqs, int start, int end)
+{
+	int i;
+
+	for (i = start; i < end; i++)
+		if (nl_expkey_one(&reqs[i]) == EXPKEY_RETRY)
+			nl_delay_expkey(&reqs[i]);
+}
+
+/*
+ * Retry the oldest deferred lookup if it is due.  Entries are queued in
+ * time order, so only the head can be ready.
+ */
+static void nl_retry_expkey(void)
+{
+	struct delayed_expkey *d = delayed_expkey;
+	struct expkey_req req;
+
+	if (!d || d->last_attempt + RETRY_SEC > time(NULL))
+		return;
+
+	delayed_expkey = d->next;
+	d->next = NULL;
+
+	req.client = d->client;
+	req.fsidtype = d->fsidtype;
+	req.fsid = d->fsid;
+	req.fsidlen = d->fsidlen;
+
+	if (nl_expkey_one(&req) == EXPKEY_RETRY)
+		delayed_expkey_enqueue(d);
+	else
+		delayed_expkey_free(d);
+}
+
 static void cache_nl_process_expkey(void)
 {
 	struct expkey_req *reqs = NULL;
 	int nreqs = 0;
-	struct nl_msg *msg;
-	int i;
+	int i = 0;
 
 	if (cache_nl_get_expkey_reqs(&reqs, &nreqs)) {
 		xlog(L_WARNING, "cache_nl_process_expkey: failed to get expkey requests");
@@ -2178,116 +2394,41 @@ static void cache_nl_process_expkey(void)
 
 	xlog(D_CALL, "cache_nl_process_expkey: %d pending expkey requests", nreqs);
 
-	msg = cache_nl_new_msg(nfsd_nl_family, NFSD_CMD_EXPKEY_SET_REQS, 0);
-	if (!msg)
-		goto out_free;
-
-	for (i = 0; i < nreqs; i++) {
-		char *dom = reqs[i].client;
-		int fsidtype = reqs[i].fsidtype;
-		char *fsid = reqs[i].fsid;
-		int fsidlen = reqs[i].fsidlen;
-		struct parsed_fsid parsed;
-		struct addrinfo *ai = NULL;
-		struct exportent *found = NULL;
-		char *found_path = NULL;
-		nfs_export *exp;
-		int j;
-
-		if (parse_fsid(fsidtype, fsidlen, fsid, &parsed))
-			goto do_add_expkey;
-
-		if (is_ipaddr_client(dom)) {
-			ai = lookup_client_addr(dom);
-			if (!ai)
-				goto do_add_expkey;
-		}
-
-		for (j = 0; j < MCL_MAXTYPES; j++) {
-			nfs_export *prev = NULL;
-			nfs_export *next_exp;
-			void *mnt = NULL;
-
-			for (exp = exportlist[j].p_head; exp;
-			     exp = next_exp) {
-				char *path;
-
-				if (exp->m_export.e_flags &
-				    NFSEXP_CROSSMOUNT) {
-					if (prev == exp) {
-						path = next_mnt(&mnt,
-							exp->m_export.e_path);
-						if (!path) {
-							next_exp = exp->m_next;
-							prev = NULL;
-							continue;
-						}
-						next_exp = exp;
-					} else {
-						prev = exp;
-						mnt = NULL;
-						path = exp->m_export.e_path;
-						next_exp = exp;
-					}
-				} else {
-					path = exp->m_export.e_path;
-					next_exp = exp->m_next;
-				}
-
-				if (!is_ipaddr_client(dom) &&
-				    !namelist_client_matches(exp, dom))
-					continue;
+	while (i < nreqs) {
+		int start = i;
+		struct nl_msg *msg;
 
-				switch (match_fsid(&parsed, exp, path)) {
-				case 0:
-					continue;
-				case -1:
-					continue;
-				}
+		msg = cache_nl_new_msg(nfsd_nl_family,
+				       NFSD_CMD_EXPKEY_SET_REQS, 0);
+		if (!msg)
+			break;
 
-				if (is_ipaddr_client(dom) &&
-				    !ipaddr_client_matches(exp, ai))
-					continue;
+		for (; i < nreqs; i++) {
+			enum expkey_result res;
 
-				if (!found ||
-				    subexport(&exp->m_export, found)) {
-					found = &exp->m_export;
-					free(found_path);
-					found_path = strdup(path);
-					if (!found_path)
-						goto do_add_expkey;
-				}
-			}
+			res = nl_add_expkey_req(msg, &reqs[i]);
+			if (res == EXPKEY_FULL)
+				break;
+			if (res == EXPKEY_RETRY)
+				nl_delay_expkey(&reqs[i]);
 		}
 
-do_add_expkey:
-		if (nfsd_nl_add_expkey(msg, dom, fsidtype, fsid,
-				       fsidlen, found_path) < 0) {
-			cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
-			nlmsg_free(msg);
-			msg = cache_nl_new_msg(nfsd_nl_family,
-					       NFSD_CMD_EXPKEY_SET_REQS, 0);
-			if (!msg) {
-				free(found_path);
-				nfs_freeaddrinfo(ai);
-				goto out_free;
-			}
-			if (nfsd_nl_add_expkey(msg, dom, fsidtype, fsid,
-					       fsidlen, found_path) < 0)
-				xlog(L_WARNING, "%s: skipping oversized "
-				     "entry", __func__);
+		/*
+		 * One bad entry fails the whole message, so resubmit the
+		 * batch singly to find out which and answer the rest.
+		 */
+		if (nl_msg_has_reqs(msg) &&
+		    cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL) < 0)
+			nl_expkey_singly(reqs, start, i);
+		nlmsg_free(msg);
+
+		/* First entry did not fit an empty message: answer it alone */
+		if (i == start) {
+			nl_expkey_singly(reqs, i, i + 1);
+			i++;
 		}
-		if (!found)
-			xlog(D_AUTH, "denied access to %s",
-			     *dom == '$' ? dom + 1 : dom);
-		free(found_path);
-		nfs_freeaddrinfo(ai);
 	}
 
-	cache_nl_set_reqs(nfsd_nl_cmd_sock, msg, NULL);
-	nlmsg_free(msg);
-
-out_free:
 	for (i = 0; i < nreqs; i++) {
 		free(reqs[i].client);
 		free(reqs[i].fsid);
@@ -3487,12 +3628,15 @@ int cache_process(fd_set *readfds)
 	cache_set_fds(readfds);
 	v4clients_set_fds(readfds);
 
-	if (delayed || delayed_export) {
+	if (delayed || delayed_expkey || delayed_export) {
 		time_t now = time(NULL);
 		time_t delay = RETRY_SEC;
 
 		if (delayed)
 			delay = retry_delay(&delayed->last_attempt, now, delay);
+		if (delayed_expkey)
+			delay = retry_delay(&delayed_expkey->last_attempt, now,
+					    delay);
 		if (delayed_export)
 			delay = retry_delay(&delayed_export->last_attempt, now,
 					    delay);
@@ -3513,6 +3657,7 @@ int cache_process(fd_set *readfds)
 		}
 	}
 
+	nl_retry_expkey();
 	nl_retry_export();
 
 	switch (selret) {

-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-09 12:56 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09 12:56 [PATCH nfs-utils 0/5] mountd: fixes for netlink up/downcall interfaces Jeff Layton
2026-09-09 12:56 ` [PATCH nfs-utils 1/5] mountd: factor out the per-path export attribute computation Jeff Layton
2026-09-09 12:56 ` [PATCH nfs-utils 2/5] mountd: handle unmountable paths and junctions in the netlink downcall Jeff Layton
2026-09-09 12:56 ` [PATCH nfs-utils 3/5] mountd: answer requests the kernel rejects on " Jeff Layton
2026-09-09 12:56 ` [PATCH nfs-utils 4/5] mountd: don't leak the parent export's fsid onto crossmnt submounts Jeff Layton
2026-09-09 12:56 ` [PATCH nfs-utils 5/5] mountd: retry unresolvable fsid lookups on the netlink downcall Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox