nfsd: ensure SEQUENCE replay sends a valid reply.

nfsd4_enc_sequence_replay() uses nfsd4_encode_operation() to encode a
new SEQUENCE reply when replaying a request from the slot cache - only
ops after the SEQUENCE are replayed from the cache in ->sl_data.

However it does this in nfsd4_replay_cache_entry() which is called
*before* nfsd4_sequence() has filled in reply fields.

This means that in the replayed SEQUENCE reply:
 maxslots will be whatever the client sent
 target_maxslots will be -1 (assuming init to zero, and
      nfsd4_encode_sequence() subtracts 1)
 status_flags will be zero

The incorrect maxslots value, in particular, can cause the client to
think the slot table has been reduced in size so it can discard its
knowledge of current sequence number of the later slots, though the
server has not discarded those slots.  When the client later wants to
use a later slot, it can get NFS4ERR_SEQ_MISORDERED from the server.

This patch moves the setup of the reply into a new helper function and
call it *before* nfsd4_replay_cache_entry() is called.  Only one of the
updated fields was used after this point - maxslots.  So the
nfsd4_sequence struct has been extended to have separate maxslots for
the request and the response.

Reported-by: Olga Kornievskaia <okorniev@redhat.com>
Closes: https://lore.kernel.org/linux-nfs/20251010194449.10281-1-okorniev@redhat.com/
Tested-by: Olga Kornievskaia <okorniev@redhat.com>
Signed-off-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c8c326679dca0ad08a9b681681acb85ac3bb2699..8a6960500217412d1aa5d82102ecf6888ff2f27c 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4363,6 +4363,36 @@ static bool replay_matches_cache(struct svc_rqst *rqstp,
        return true;
 }
 
+/*
+ * Note that the response is constructed here both for the case
+ * of a new SEQUENCE request and for a replayed SEQUENCE request.
+ * We do not cache SEQUENCE responses as SEQUENCE is idempotent.
+ */
+static void nfsd4_construct_sequence_response(struct nfsd4_session *session,
+                                             struct nfsd4_sequence *seq)
+{
+       struct nfs4_client *clp = session->se_client;
+
+       seq->maxslots_response = max(session->se_target_maxslots,
+                                    seq->maxslots);
+       seq->target_maxslots = session->se_target_maxslots;
+
+       switch (clp->cl_cb_state) {
+       case NFSD4_CB_DOWN:
+               seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
+               break;
+       case NFSD4_CB_FAULT:
+               seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;
+               break;
+       default:
+               seq->status_flags = 0;
+       }
+       if (!list_empty(&clp->cl_revoked))
+               seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
+       if (atomic_read(&clp->cl_admin_revoked))
+               seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
+}
+
 __be32
 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
                union nfsd4_op_u *u)
@@ -4412,6 +4442,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        dprintk("%s: slotid %d\n", __func__, seq->slotid);
 
        trace_nfsd_slot_seqid_sequence(clp, seq, slot);
+
+       nfsd4_construct_sequence_response(session, seq);
+
        status = check_slot_seqid(seq->seqid, slot->sl_seqid, slot->sl_flags);
        if (status == nfserr_replay_cache) {
                status = nfserr_seq_misordered;
@@ -4509,23 +4542,6 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
        }
 
 out:
-       seq->maxslots = max(session->se_target_maxslots, seq->maxslots);
-       seq->target_maxslots = session->se_target_maxslots;
-
-       switch (clp->cl_cb_state) {
-       case NFSD4_CB_DOWN:
-               seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
-               break;
-       case NFSD4_CB_FAULT:
-               seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;
-               break;
-       default:
-               seq->status_flags = 0;
-       }
-       if (!list_empty(&clp->cl_revoked))
-               seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
-       if (atomic_read(&clp->cl_admin_revoked))
-               seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
        trace_nfsd_seq4_status(rqstp, seq);
 out_no_session:
        if (conn)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index cf5df0f502089a8bced51dff40d7b9fff9aa39ab..67bb9c0b9fcb183332d09fd51cdc32a3d6cac6d4 100644 (file)
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -5073,7 +5073,7 @@ nfsd4_encode_sequence(struct nfsd4_compoundres *resp, __be32 nfserr,
                return nfserr;
        /* Note slotid's are numbered from zero: */
        /* sr_highest_slotid */
-       nfserr = nfsd4_encode_slotid4(xdr, seq->maxslots - 1);
+       nfserr = nfsd4_encode_slotid4(xdr, seq->maxslots_response - 1);
        if (nfserr != nfs_ok)
                return nfserr;
        /* sr_target_highest_slotid */

diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index ee0570cbdd9e88095fd1bd12a3805795790e99c1..1ce8e12ae3354c808d2ea54988f4cf1b510e3ed7 100644 (file)
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -574,8 +574,9 @@ struct nfsd4_sequence {
        struct nfs4_sessionid   sessionid;              /* request/response */
        u32                     seqid;                  /* request/response */
        u32                     slotid;                 /* request/response */
-       u32                     maxslots;               /* request/response */
+       u32                     maxslots;               /* request */
        u32                     cachethis;              /* request */
+       u32                     maxslots_response;      /* response */
        u32                     target_maxslots;        /* response */
        u32                     status_flags;           /* response */
 };