* [PATCH 0/2] nfsd: fix a slab overwrite in the NFSv4.1 session reply cache
@ 2026-08-18 0:57 Chuck Lever
2026-08-18 0:57 ` [PATCH 1/2] nfsd: preflight SEQUENCE replies before accepting a slot Chuck Lever
2026-08-18 0:57 ` [PATCH 2/2] nfsd: set op->status when an operation's header cannot be encoded Chuck Lever
0 siblings, 2 replies; 3+ messages in thread
From: Chuck Lever @ 2026-08-18 0:57 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Jérémy Jean, Chuck Lever
A client that can establish a session can negotiate
ca_maxresponsesize_cached down to NFSD_MIN_HDR_SEQ_SZ, which leaves
every slot's sl_data[] zero bytes long, and then send a cachethis
SEQUENCE whose COMPOUND tag fills the narrowed reply buffer. The
SEQUENCE result is never encoded, cstate.data_offset stays zero, and
the whole reply is copied past the end of the slot. Jérémy Jean found
this and reported it privately, with a KUnit fixture that reproduces
the write under KASAN. The fixture is not part of this series.
nfsd4_store_cache_entry() derives the length to copy from
cstate.data_offset and never compares it against what the slot was
allocated. Clamping the copy there would stop the write, but the slot
would then hold a truncated reply that a retry replays as a complete
one, and the slot seqid has already been consumed by that point.
RFC 8881 Section 2.10.6.1.2 requires an error returned from SEQUENCE
to leave the slot untouched, so the size has to be settled before the
slot is accepted. The copy in nfsd4_store_cache_entry() is left as it
stands.
The new check runs ahead of xdr_restrict_buflen(), which fails only
once the headers and tag alone overrun the negotiated limit. Adding
the fixed-size SEQUENCE result on top of them also rejects a request
that leaves no room for the reply the client asked to have cached.
Such a request cannot produce a complete reply today either.
---
Chuck Lever (1):
nfsd: set op->status when an operation's header cannot be encoded
Jérémy Jean (1):
nfsd: preflight SEQUENCE replies before accepting a slot
fs/nfsd/nfs4state.c | 19 ++++++++++++++++++-
fs/nfsd/nfs4xdr.c | 13 +++++++++++--
2 files changed, 29 insertions(+), 3 deletions(-)
---
base-commit: 76427d869120552a1a82e1f1488d9f8311827d84
change-id: 20260815-jean-70ae7975e5a1
Best regards,
--
Chuck Lever
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] nfsd: preflight SEQUENCE replies before accepting a slot
2026-08-18 0:57 [PATCH 0/2] nfsd: fix a slab overwrite in the NFSv4.1 session reply cache Chuck Lever
@ 2026-08-18 0:57 ` Chuck Lever
2026-08-18 0:57 ` [PATCH 2/2] nfsd: set op->status when an operation's header cannot be encoded Chuck Lever
1 sibling, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2026-08-18 0:57 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Jérémy Jean, Chuck Lever
From: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
nfsd4_sequence() narrows the reply buffer to the session's cached
reply limit before it accepts the slot seqid. A client may negotiate
ca_maxresponsesize_cached down to NFSD_MIN_HDR_SEQ_SZ, and
nfsd4_alloc_slot() then gives every slot a zero-length sl_data[]. A
COMPOUND tag can fill that narrowed buffer until it holds the
SEQUENCE opcode but not the status word that follows.
nfsd4_encode_operation() returns without running
nfsd4_encode_sequence(), so cstate.data_offset stays zero. It leaves
op->status at nfs_ok as well, so the COMPOUND is treated as having
succeeded.
nfsd4_store_cache_entry() declines to cache a lone SEQUENCE that
returned an error. That test reads the status the operation
reported, so it passes here. The copy starts at offset zero and
takes the whole reply, RPC and COMPOUND headers included, into the
zero-length sl_data[]. The COMPOUND tag is copied along with it, so
the client picks most of the bytes written past the end of the slot:
BUG: KASAN: slab-out-of-bounds in read_bytes_from_xdr_buf+0x1bc/0x390
Write of size 80 at addr ffff888003a549cd by task kunit_try_catch/24
__asan_memcpy+0x38/0x60
read_bytes_from_xdr_buf+0x1bc/0x390
nfsd4_sequence_done+0x5b0/0x810
nfs4svc_encode_compoundres+0x1bf/0x240
Check that the fixed-size SEQUENCE result, plus room for a following
operation's error status, fits the negotiated limit before narrowing
the buffer and consuming the slot seqid. The slot and its reply
cache are left unchanged, as RFC 8881 Section 2.10.6.1.2 requires of
an error returned from SEQUENCE.
Fixes: 47ee52986472 ("nfsd4: adjust buflen to session channel limit")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4state.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1ba97e3f65eb..3fc5bed85bab 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5044,6 +5044,7 @@ __be32
nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
union nfsd4_op_u *u)
{
+ struct nfsd4_compoundargs *args = rqstp->rq_argp;
struct nfsd4_sequence *seq = &u->sequence;
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct xdr_stream *xdr = resp->xdr;
@@ -5053,6 +5054,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct nfsd4_conn *conn;
__be32 status;
int buflen;
+ u32 maxlen, respsize;
struct net *net = SVC_NET(rqstp);
struct nfsd_net *nn = net_generic(net, nfsd_net_id);
@@ -5130,7 +5132,22 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
session->se_fchannel.maxresp_sz;
status = (seq->cachethis) ? nfserr_rep_too_big_to_cache :
nfserr_rep_too_big;
- if (xdr_restrict_buflen(xdr, buflen - rqstp->rq_auth_slack))
+ if (buflen < rqstp->rq_auth_slack)
+ goto out_put_session;
+ maxlen = buflen - rqstp->rq_auth_slack;
+
+ /*
+ * A SEQUENCE result too large for maxlen never reaches
+ * nfsd4_encode_sequence(), so cstate.data_offset stays zero and
+ * the reply cache overruns the slot.
+ */
+ respsize = nfsd4_max_reply(rqstp, &args->ops[0]);
+ if (!nfsd4_last_compound_op(rqstp))
+ respsize += COMPOUND_ERR_SLACK_SPACE;
+ if (xdr->buf->len + respsize > maxlen)
+ goto out_put_session;
+
+ if (xdr_restrict_buflen(xdr, maxlen))
goto out_put_session;
svc_reserve_auth(rqstp, buflen);
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] nfsd: set op->status when an operation's header cannot be encoded
2026-08-18 0:57 [PATCH 0/2] nfsd: fix a slab overwrite in the NFSv4.1 session reply cache Chuck Lever
2026-08-18 0:57 ` [PATCH 1/2] nfsd: preflight SEQUENCE replies before accepting a slot Chuck Lever
@ 2026-08-18 0:57 ` Chuck Lever
1 sibling, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2026-08-18 0:57 UTC (permalink / raw)
To: Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo, Tom Talpey
Cc: linux-nfs, Chuck Lever
nfsd4_encode_operation() leaves op->status alone when the reply
buffer has no room for the operation's opcode and status word.
nfsd4_proc_compound() reads the unchanged nfs_ok as success and
goes on to the next operation, so the reply counts an operation
whose result was never encoded.
Report the failure through nfsd4_check_resp_size(), which the rest
of the function already uses. It returns NFS4ERR_REP_TOO_BIG, or
NFS4ERR_REP_TOO_BIG_TO_CACHE on a session, and the COMPOUND ends
at that operation.
Two paths narrow the reply buffer: nfsd4_sequence(), which rejects
a SEQUENCE result that does not fit, and nfsd4_encode_splice_read(),
which can leave a single XDR word in the head page. Whether a
COMPOUND reaches that boundary is unproven, so this is a guard
rather than a fix.
Signed-off-by: Chuck Lever <cel@kernel.org>
---
fs/nfsd/nfs4xdr.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 7d1b2d6f57f2..a154b02d82b3 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -6723,11 +6723,20 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
unsigned int op_status_offset;
nfsd4_enc encoder;
- if (xdr_stream_encode_u32(xdr, op->opnum) != XDR_UNIT)
+ /*
+ * nfsd4_proc_compound() stops the COMPOUND early only
+ * when op->status is set, so a header that cannot be
+ * encoded has to report the failure here.
+ */
+ if (xdr_stream_encode_u32(xdr, op->opnum) != XDR_UNIT) {
+ op->status = nfsd4_check_resp_size(resp, XDR_UNIT * 2);
goto release;
+ }
op_status_offset = xdr->buf->len;
- if (!xdr_reserve_space(xdr, XDR_UNIT))
+ if (!xdr_reserve_space(xdr, XDR_UNIT)) {
+ op->status = nfsd4_check_resp_size(resp, XDR_UNIT);
goto release;
+ }
if (op->opnum == OP_ILLEGAL)
goto status;
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-18 0:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-18 0:57 [PATCH 0/2] nfsd: fix a slab overwrite in the NFSv4.1 session reply cache Chuck Lever
2026-08-18 0:57 ` [PATCH 1/2] nfsd: preflight SEQUENCE replies before accepting a slot Chuck Lever
2026-08-18 0:57 ` [PATCH 2/2] nfsd: set op->status when an operation's header cannot be encoded Chuck Lever
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox