[PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY

[PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY

[Jeff Layton]

I posted a patch to fix up a sparse warning with a cast a few days ago
[1], but Chuck objected and wanted to fix this by moving these
(host-endian) allocations out of the xdr stream entirely.

This patchset should do that. Some of them are able to be moved to
temporary stack allocations, but the mask fields need a separate
kmalloc'ed field tracked in struct nfsd4_cb_notify.

Please consider these for v7.4.

[1]: https://lore.kernel.org/linux-nfs/20260803-dir-deleg-v1-1-51be76861821@kernel.org/

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (2):
      nfsd: pass caller-provided attrmask storage into nfsd4_setup_notify_entry4()
      nfsd: back CB_NOTIFY notify_mask words with per-delegation storage

 fs/nfsd/nfs4state.c | 20 +++++++++-----------
 fs/nfsd/nfs4xdr.c   | 28 ++++++++++++++--------------
 fs/nfsd/state.h     |  1 +
 3 files changed, 24 insertions(+), 25 deletions(-)
---
base-commit: d33023dfd81b31e80fe65c2921993b1a6cd7043c
change-id: 20260812-dir-deleg-98f3bdcb8223

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

[PATCH 1/2] nfsd: pass caller-provided attrmask storage into nfsd4_setup_notify_entry4()

[Jeff Layton]

nfsd4_setup_notify_entry4() stole 3 words from the xdr stream via
xdr_reserve_space() to hold the host-order bmval[3] attrmask that
nfsd4_encode_attr_vals() consumes and ne_attrs.attrmask.element points
at. Stashing host-endian scratch in an XDR stream buffer is fragile:
the buffer layout is not guaranteed by sunrpc, and it blocks moving the
encoder to pages or xdrgen.

The attrmask only needs to live until the enclosing encode call
serializes the notify_entry4, so hand it caller-provided stack storage
instead. The two callers keep the words on the stack: up to three
concurrent entries for a rename in nfsd4_encode_notify_event(), one in
nfsd4_encode_dir_attr_change().

No wire change: the reserved words were never emitted; attr_vals.data/len
are still captured relative to xdr->p.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4xdr.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index a47eb544b99f..7d1b2d6f57f2 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4378,21 +4378,16 @@ setup_notify_fhandle(struct dentry *dentry, struct nfs4_delegation *dp,
 static bool
 nfsd4_setup_notify_entry4(struct notify_entry4 *ne, struct xdr_stream *xdr,
 			  struct dentry *dentry, struct nfs4_delegation *dp,
-			  struct nfsd_file *nf, char *name, u32 namelen)
+			  struct nfsd_file *nf, char *name, u32 namelen,
+			  u32 *attrmask)
 {
 	struct path path = nf->nf_file->f_path;
 	struct nfsd4_fattr_args args = { };
 	const u32 *reqmask;
-	uint32_t *attrmask;
 	__be32 status;
 	bool parent;
 	int ret;
 
-	/* Reserve space for attrmask */
-	attrmask = xdr_reserve_space(xdr, 3 * sizeof(uint32_t));
-	if (!attrmask)
-		return false;
-
 	ne->ne_file.data = name;
 	ne->ne_file.len = namelen;
 	ne->ne_attrs.attrmask.element = attrmask;
@@ -4476,6 +4471,7 @@ u8 *nfsd4_encode_notify_event(struct xdr_stream *xdr, struct nfsd_notify_event *
 			      struct nfs4_delegation *dp, struct nfsd_file *nf,
 			      u32 *notify_mask)
 {
+	u32 attrmask[3][3] = { };
 	u8 *p = NULL;
 
 	*notify_mask = 0;
@@ -4484,7 +4480,8 @@ u8 *nfsd4_encode_notify_event(struct xdr_stream *xdr, struct nfsd_notify_event *
 		struct notify_remove4 nr = { };
 
 		if (!nfsd4_setup_notify_entry4(&nr.nrm_old_entry, xdr, nne->ne_dentry, dp,
-					       nf, nne->ne_name, nne->ne_namelen))
+					       nf, nne->ne_name, nne->ne_namelen,
+					       attrmask[0]))
 			goto out_err;
 		p = (u8 *)xdr->p;
 		if (!xdrgen_encode_notify_remove4(xdr, &nr))
@@ -4495,14 +4492,16 @@ u8 *nfsd4_encode_notify_event(struct xdr_stream *xdr, struct nfsd_notify_event *
 		struct notify_remove4 old = { };
 
 		if (!nfsd4_setup_notify_entry4(&na.nad_new_entry, xdr, nne->ne_dentry, dp,
-					       nf, nne->ne_name, nne->ne_namelen))
+					       nf, nne->ne_name, nne->ne_namelen,
+					       attrmask[0]))
 			goto out_err;
 
 		/* If a file was overwritten, report it in nad_old_entry */
 		if (nne->ne_target) {
 			if (!nfsd4_setup_notify_entry4(&old.nrm_old_entry, xdr,
 						       NULL, dp, nf,
-						       nne->ne_name, nne->ne_namelen))
+						       nne->ne_name, nne->ne_namelen,
+						       attrmask[1]))
 				goto out_err;
 			na.nad_old_entry.count = 1;
 			na.nad_old_entry.element = &old;
@@ -4521,19 +4520,19 @@ u8 *nfsd4_encode_notify_event(struct xdr_stream *xdr, struct nfsd_notify_event *
 		/* Don't send any attributes in the old_entry since they're the same in new */
 		if (!nfsd4_setup_notify_entry4(&nr.nrn_old_entry.nrm_old_entry, xdr,
 					       NULL, dp, nf, nne->ne_name,
-					       nne->ne_namelen))
+					       nne->ne_namelen, attrmask[0]))
 			goto out_err;
 
 		if (!nfsd4_setup_notify_entry4(&nr.nrn_new_entry.nad_new_entry, xdr,
 					       nne->ne_dentry, dp, nf, newname,
-					       nne->ne_newnamelen))
+					       nne->ne_newnamelen, attrmask[1]))
 			goto out_err;
 
 		/* If a file was overwritten, report it in nad_old_entry */
 		if (nne->ne_target) {
 			if (!nfsd4_setup_notify_entry4(&old.nrm_old_entry, xdr,
 						       NULL, dp, nf, newname,
-						       nne->ne_newnamelen))
+						       nne->ne_newnamelen, attrmask[2]))
 				goto out_err;
 			nr.nrn_new_entry.nad_old_entry.count = 1;
 			nr.nrn_new_entry.nad_old_entry.element = &old;
@@ -4569,11 +4568,12 @@ u8 *nfsd4_encode_dir_attr_change(struct xdr_stream *xdr, struct nfs4_delegation
 {
 	struct dentry *dentry = nf->nf_file->f_path.dentry;
 	struct notify_attr4 na = { };
+	u32 attrmask[3] = { };
 	u8 *p;
 
 	/* RFC 8881 s10.4.3: ne_file must be a zero-length string for dir attrs */
 	if (!nfsd4_setup_notify_entry4(&na.na_changed_entry, xdr,
-				       dentry, dp, nf, "", 0))
+				       dentry, dp, nf, "", 0, attrmask))
 		return ERR_PTR(-ENOBUFS);
 
 	/* No requested attributes to report; omit the event */

-- 
2.55.0

[PATCH 2/2] nfsd: back CB_NOTIFY notify_mask words with per-delegation storage

[Jeff Layton]

nfsd4_cb_notify_prepare() reserved a word from the encoding xdr stream
for each notify4's host-order notify_mask, storing the pointer in
ncn_nf[].notify_mask.element. That element must survive until the RPC
encode re-reads ncn_nf, so the host-endian word lived inside the XDR
staging buffer for the whole callback lifetime - the same fragile
pattern as the attrmask, and one that keeps host-order bytes in a
buffer meant to hold big-endian XDR.

ncn_nf is a bounded per-delegation array reused across every CB_NOTIFY,
so give it a parallel ncn_masks array with the same lifetime:

- allocate/free ncn_masks alongside ncn_nf in alloc_init_dir_deleg() /
  nfs4_free_dir_deleg()
- point notify_mask.element at &ncn_masks[i] (events) and
  &ncn_masks[count] (dir attr change)

The mask backing is now pre-allocated, so the per-word NULL checks in
prepare go away. The staging stream holds only encoded XDR.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 20 +++++++++-----------
 fs/nfsd/state.h     |  1 +
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 510380b6aa7a..1ba97e3f65eb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -1323,6 +1323,7 @@ static void nfs4_free_dir_deleg(struct nfs4_stid *stid)
 	for (i = 0; i < ncn->ncn_evt_cnt; ++i)
 		nfsd_notify_event_put(ncn->ncn_evt[i]);
 	kfree(ncn->ncn_nf);
+	kfree(ncn->ncn_masks);
 	for (i = 0; i < NOTIFY4_PAGE_ARRAY_SIZE; i++) {
 		if (!ncn->ncn_pages[i])
 			break;
@@ -1355,6 +1356,11 @@ alloc_init_dir_deleg(struct nfs4_client *clp, struct nfs4_file *fp)
 		nfs4_put_stid(&dp->dl_stid);
 		return NULL;
 	}
+	ncn->ncn_masks = kcalloc(NOTIFY4_EVENT_QUEUE_SIZE, sizeof(*ncn->ncn_masks), GFP_KERNEL);
+	if (!ncn->ncn_masks) {
+		nfs4_put_stid(&dp->dl_stid);
+		return NULL;
+	}
 	spin_lock_init(&ncn->ncn_lock);
 	nfsd4_init_cb(&ncn->ncn_cb, dp->dl_stid.sc_client,
 			&nfsd4_cb_notify_ops, NFSPROC4_CLNT_CB_NOTIFY);
@@ -3767,14 +3773,9 @@ nfsd4_cb_notify_prepare(struct nfsd4_callback *cb)
 		struct nfsd_notify_event *nne = events[i];
 
 		if (!error) {
-			u32 *maskp = (u32 *)xdr_reserve_space(&stream, sizeof(*maskp));
+			u32 *maskp = &ncn->ncn_masks[i];
 			u8 *p;
 
-			if (!maskp) {
-				error = true;
-				goto put_event;
-			}
-
 			p = nfsd4_encode_notify_event(&stream, nne, dp, nf, maskp);
 			if (!p) {
 				pr_notice("Could not generate CB_NOTIFY from fsnotify mask 0x%x\n",
@@ -3792,13 +3793,10 @@ nfsd4_cb_notify_prepare(struct nfsd4_callback *cb)
 		nfsd_notify_event_put(nne);
 	}
 	if (!error && (dp->dl_notify_mask & BIT(NOTIFY4_CHANGE_DIR_ATTRS))) {
-		u32 *maskp = (u32 *)xdr_reserve_space(&stream, sizeof(*maskp));
+		u32 *maskp = &ncn->ncn_masks[count];
 		u8 *p;
 
-		if (maskp)
-			p = nfsd4_encode_dir_attr_change(&stream, dp, nf);
-		else
-			p = ERR_PTR(-ENOBUFS);
+		p = nfsd4_encode_dir_attr_change(&stream, dp, nf);
 
 		if (IS_ERR(p)) {
 			/*
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index ff1c9fa731aa..c65b604e29f1 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -271,6 +271,7 @@ struct nfsd4_cb_notify {
 	struct nfsd_notify_event	*ncn_evt[NOTIFY4_EVENT_QUEUE_SIZE]; // list of events
 	struct page			*ncn_pages[NOTIFY4_PAGE_ARRAY_SIZE]; // for encoding
 	struct notify4			*ncn_nf;	// array of notify4's to be sent
+	u32				*ncn_masks;	// host-order notify_mask backing for ncn_nf[]
 	bool				ncn_encode_err;	// did encoding fail?
 	struct nfsd4_callback		ncn_cb;		// notify4 callback
 };

-- 
2.55.0

Re: [PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY

[Chuck Lever]

On Wed, 12 Aug 2026 14:08:13 -0400, Jeff Layton wrote:
> I posted a patch to fix up a sparse warning with a cast a few days ago
> [1], but Chuck objected and wanted to fix this by moving these
> (host-endian) allocations out of the xdr stream entirely.
> 
> This patchset should do that. Some of them are able to be moved to
> temporary stack allocations, but the mask fields need a separate
> kmalloc'ed field tracked in struct nfsd4_cb_notify.
> 
> [...]

Applied to nfsd-testing, thanks!

[1/2] nfsd: pass caller-provided attrmask storage into nfsd4_setup_notify_entry4()
      commit: fced96df83e491cc182e9ec3ac6cf9234e705410
[2/2] nfsd: back CB_NOTIFY notify_mask words with per-delegation storage
      commit: de7051270554f99559f9b6bcf3c4fb695c7c223d

--
Chuck Lever