* [PATCH 1/2] nfsd: pass caller-provided attrmask storage into nfsd4_setup_notify_entry4()
2026-08-12 18:08 [PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY Jeff Layton
@ 2026-08-12 18:08 ` Jeff Layton
2026-08-12 18:08 ` [PATCH 2/2] nfsd: back CB_NOTIFY notify_mask words with per-delegation storage Jeff Layton
2026-08-12 18:58 ` [PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY Chuck Lever
2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2026-08-12 18:08 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, linux-kernel, 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
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] nfsd: back CB_NOTIFY notify_mask words with per-delegation storage
2026-08-12 18:08 [PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY Jeff Layton
2026-08-12 18:08 ` [PATCH 1/2] nfsd: pass caller-provided attrmask storage into nfsd4_setup_notify_entry4() Jeff Layton
@ 2026-08-12 18:08 ` Jeff Layton
2026-08-12 18:58 ` [PATCH 0/2] nfsd: don't use the xdr buf for temporary storage in CB_NOTIFY Chuck Lever
2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2026-08-12 18:08 UTC (permalink / raw)
To: Chuck Lever, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, linux-kernel, 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
^ permalink raw reply [flat|nested] 4+ messages in thread