[PATCH] nfsd: fix replay buffer length underflow in nfsd4_encode_operation

[PATCH] nfsd: fix replay buffer length underflow in nfsd4_encode_operation

[Xiaobo Liu]

When nfsd4_encode_operation() truncates the reply back to
op_status_offset + XDR_UNIT, the replay-cache path may still try to
compute the encoded payload length from xdr->buf->len.

If xdr->buf->len is smaller than op_status_offset + XDR_UNIT, the
subtraction underflows and the result is assigned to an int. That can
produce an invalid negative/small value, allowing the subsequent
NFSD4_REPLAY_ISIZE check to make the wrong decision and use a bogus
length for replay-buffer handling.

Fix this by validating the buffer length before subtracting. If the
encoded length would underflow, force the value into the "too large to
cache" path so rp_buflen is cleared instead of reading invalid data.

Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
---
 fs/nfsd/nfs4xdr.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9d2349131..5e9e49057 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -6278,9 +6278,14 @@ nfsd4_encode_operation(struct nfsd4_compoundres *resp, struct nfsd4_op *op)
 		warn_on_nonidempotent_op(op);
 		xdr_truncate_encode(xdr, op_status_offset + XDR_UNIT);
 	} else if (so) {
-		int len = xdr->buf->len - (op_status_offset + XDR_UNIT);
+		unsigned int len;
 
 		so->so_replay.rp_status = op->status;
+		if (xdr->buf->len >= op_status_offset + XDR_UNIT) {
+			len = xdr->buf->len - (op_status_offset + XDR_UNIT);
+		} else {
+			len = NFSD4_REPLAY_ISIZE + 1;
+		}
 		if (len <= NFSD4_REPLAY_ISIZE) {
 			so->so_replay.rp_buflen = len;
 			read_bytes_from_xdr_buf(xdr->buf,
-- 
2.34.1

Re: [PATCH] nfsd: fix replay buffer length underflow in nfsd4_encode_operation

[Chuck Lever]

On Sun, Apr 12, 2026, at 6:01 AM, Xiaobo Liu wrote:
> When nfsd4_encode_operation() truncates the reply back to
> op_status_offset + XDR_UNIT, the replay-cache path may still try to
> compute the encoded payload length from xdr->buf->len.

It seems to me that this sequence cannot actually occur.  The
xdr_truncate_encode() call and the replay-cache length computation
are in mutually exclusive branches of an if/else-if chain in
nfsd4_encode_operation():

  if (op->status == nfserr_resource ||
      op->status == nfserr_rep_too_big ||
      op->status == nfserr_rep_too_big_to_cache) {
          ...
          xdr_truncate_encode(xdr, op_status_offset + XDR_UNIT);
  } else if (so) {
          /* replay-cache length computation here */
  }

The replay-cache path only executes when the truncation path does
not. The commit message describes a flow where both execute, but
the else-if prevents that.


> If xdr->buf->len is smaller than op_status_offset + XDR_UNIT, the
> subtraction underflows

Is this condition reachable?  op_status_offset is captured from
xdr->buf->len before xdr_reserve_space(xdr, XDR_UNIT) at the top
of the function. After the reserve succeeds, buf->len is at least
op_status_offset + XDR_UNIT. The encoder called at line

  op->status = encoder(resp, op->status, &op->u);

only adds data to the buffer. Encoders that internally truncate
(e.g. nfsd4_encode_readdir) truncate to their own starting_len,
which is recorded after the opnum and status are already encoded,
so buf->len stays >= op_status_offset + XDR_UNIT. After
xdr_commit_encode(), the invariant still holds.

The underflow condition appears structurally impossible on this
code path.


-- 
Chuck Lever