Skip to content

Commit 1cff14b

Browse files
neilbrownchucklever
authored andcommitted
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>
1 parent c96573c commit 1cff14b

3 files changed

Lines changed: 36 additions & 19 deletions

File tree

fs/nfsd/nfs4state.c

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4363,6 +4363,36 @@ static bool replay_matches_cache(struct svc_rqst *rqstp,
43634363
return true;
43644364
}
43654365

4366+
/*
4367+
* Note that the response is constructed here both for the case
4368+
* of a new SEQUENCE request and for a replayed SEQUENCE request.
4369+
* We do not cache SEQUENCE responses as SEQUENCE is idempotent.
4370+
*/
4371+
static void nfsd4_construct_sequence_response(struct nfsd4_session *session,
4372+
struct nfsd4_sequence *seq)
4373+
{
4374+
struct nfs4_client *clp = session->se_client;
4375+
4376+
seq->maxslots_response = max(session->se_target_maxslots,
4377+
seq->maxslots);
4378+
seq->target_maxslots = session->se_target_maxslots;
4379+
4380+
switch (clp->cl_cb_state) {
4381+
case NFSD4_CB_DOWN:
4382+
seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
4383+
break;
4384+
case NFSD4_CB_FAULT:
4385+
seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;
4386+
break;
4387+
default:
4388+
seq->status_flags = 0;
4389+
}
4390+
if (!list_empty(&clp->cl_revoked))
4391+
seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
4392+
if (atomic_read(&clp->cl_admin_revoked))
4393+
seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
4394+
}
4395+
43664396
__be32
43674397
nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
43684398
union nfsd4_op_u *u)
@@ -4412,6 +4442,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
44124442
dprintk("%s: slotid %d\n", __func__, seq->slotid);
44134443

44144444
trace_nfsd_slot_seqid_sequence(clp, seq, slot);
4445+
4446+
nfsd4_construct_sequence_response(session, seq);
4447+
44154448
status = check_slot_seqid(seq->seqid, slot->sl_seqid, slot->sl_flags);
44164449
if (status == nfserr_replay_cache) {
44174450
status = nfserr_seq_misordered;
@@ -4509,23 +4542,6 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
45094542
}
45104543

45114544
out:
4512-
seq->maxslots = max(session->se_target_maxslots, seq->maxslots);
4513-
seq->target_maxslots = session->se_target_maxslots;
4514-
4515-
switch (clp->cl_cb_state) {
4516-
case NFSD4_CB_DOWN:
4517-
seq->status_flags = SEQ4_STATUS_CB_PATH_DOWN;
4518-
break;
4519-
case NFSD4_CB_FAULT:
4520-
seq->status_flags = SEQ4_STATUS_BACKCHANNEL_FAULT;
4521-
break;
4522-
default:
4523-
seq->status_flags = 0;
4524-
}
4525-
if (!list_empty(&clp->cl_revoked))
4526-
seq->status_flags |= SEQ4_STATUS_RECALLABLE_STATE_REVOKED;
4527-
if (atomic_read(&clp->cl_admin_revoked))
4528-
seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
45294545
trace_nfsd_seq4_status(rqstp, seq);
45304546
out_no_session:
45314547
if (conn)

fs/nfsd/nfs4xdr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5073,7 +5073,7 @@ nfsd4_encode_sequence(struct nfsd4_compoundres *resp, __be32 nfserr,
50735073
return nfserr;
50745074
/* Note slotid's are numbered from zero: */
50755075
/* sr_highest_slotid */
5076-
nfserr = nfsd4_encode_slotid4(xdr, seq->maxslots - 1);
5076+
nfserr = nfsd4_encode_slotid4(xdr, seq->maxslots_response - 1);
50775077
if (nfserr != nfs_ok)
50785078
return nfserr;
50795079
/* sr_target_highest_slotid */

fs/nfsd/xdr4.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,8 +574,9 @@ struct nfsd4_sequence {
574574
struct nfs4_sessionid sessionid; /* request/response */
575575
u32 seqid; /* request/response */
576576
u32 slotid; /* request/response */
577-
u32 maxslots; /* request/response */
577+
u32 maxslots; /* request */
578578
u32 cachethis; /* request */
579+
u32 maxslots_response; /* response */
579580
u32 target_maxslots; /* response */
580581
u32 status_flags; /* response */
581582
};

0 commit comments

Comments
 (0)