NFSD: Fix the NFSv4.1 CREATE_SESSION operation

RFC 8881 Section 18.36.4 discusses the implementation of the NFSv4.1
CREATE_SESSION operation. The section defines four phases of
operation.

Phase 2 processes the CREATE_SESSION sequence ID. As a separate
step, Phase 3 evaluates the CREATE_SESSION arguments.

The problem we are concerned with is when phase 2 is successful but
phase 3 fails. The spec language in this case is "No changes are
made to any client records on the server."

RFC 8881 Section 18.35.4 defines a "client record", and it does
/not/ contain any details related to the special CREATE_SESSION
slot. Therefore NFSD is incorrect to skip incrementing the
CREATE_SESSION sequence id when phase 3 (see Section 18.36.4) of
CREATE_SESSION processing fails. In other words, even though NFSD
happens to store the cs_slot in a client record, in terms of the
protocol the slot is logically separate from the client record.

Three complications:

1. The world has moved on since commit 86c3e16cc7aa ("nfsd4: confirm
   only on succesful create_session") broke this. So we can't simply
   revert that commit.

2. NFSD's CREATE_SESSION implementation does not cleanly delineate
   the logic of phases 2 and 3. So this won't be a surgical fix.

3. Because of the way it currently handles the CREATE_SESSION slot
   sequence number, nfsd4_create_session() isn't caching error
   responses in the CREATE_SESSION slot. Instead of replaying the
   response cache in those cases, it's executing the transaction
   again.

Reorganize the CREATE_SESSION slot sequence number accounting. This
requires that error responses are appropriately cached in the
CREATE_SESSION slot (once it is found).

Reported-by: Connor Smith <connor.smith@hitachivantara.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218382
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 79c14148ae0de4039c503289de3a3d08321f22d1..8ffa6d548a9a94bb376b0ed4a175e0168f99d2dc 100644 (file)
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -3562,6 +3562,9 @@ out_new:
        new->cl_spo_must_allow.u.words[0] = exid->spo_must_allow[0];
        new->cl_spo_must_allow.u.words[1] = exid->spo_must_allow[1];
 
+       /* Contrived initial CREATE_SESSION response */
+       new->cl_cs_slot.sl_status = nfserr_seq_misordered;
+
        add_to_unconfirmed(new);
        swap(new, conf);
 out_copy:
@@ -3732,10 +3735,10 @@ nfsd4_create_session(struct svc_rqst *rqstp,
        struct nfsd4_create_session *cr_ses = &u->create_session;
        struct sockaddr *sa = svc_addr(rqstp);
        struct nfs4_client *conf, *unconf;
+       struct nfsd4_clid_slot *cs_slot;
        struct nfs4_client *old = NULL;
        struct nfsd4_session *new;
        struct nfsd4_conn *conn;
-       struct nfsd4_clid_slot *cs_slot = NULL;
        __be32 status = 0;
        struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
 
@@ -3761,50 +3764,51 @@ nfsd4_create_session(struct svc_rqst *rqstp,
        spin_lock(&nn->client_lock);
        unconf = find_unconfirmed_client(&cr_ses->clientid, true, nn);
        conf = find_confirmed_client(&cr_ses->clientid, true, nn);
-       WARN_ON_ONCE(conf && unconf);
+       if (!conf && !unconf) {
+               status = nfserr_stale_clientid;
+               goto out_free_conn;
+       }
 
-       if (conf) {
-               status = nfserr_wrong_cred;
-               if (!nfsd4_mach_creds_match(conf, rqstp))
-                       goto out_free_conn;
+       if (conf)
                cs_slot = &conf->cl_cs_slot;
-               status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
-               if (status) {
-                       if (status == nfserr_replay_cache)
-                               status = nfsd4_replay_create_session(cr_ses, cs_slot);
+       else
+               cs_slot = &unconf->cl_cs_slot;
+       status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
+       if (status) {
+               if (status == nfserr_replay_cache) {
+                       status = nfsd4_replay_create_session(cr_ses, cs_slot);
                        goto out_free_conn;
                }
-       } else if (unconf) {
+               goto out_cache_error;
+       }
+       cs_slot->sl_seqid++;
+       cr_ses->seqid = cs_slot->sl_seqid;
+
+       if (conf) {
+               status = nfserr_wrong_cred;
+               if (!nfsd4_mach_creds_match(conf, rqstp))
+                       goto out_cache_error;
+       } else {
                status = nfserr_clid_inuse;
                if (!same_creds(&unconf->cl_cred, &rqstp->rq_cred) ||
                    !rpc_cmp_addr(sa, (struct sockaddr *) &unconf->cl_addr)) {
                        trace_nfsd_clid_cred_mismatch(unconf, rqstp);
-                       goto out_free_conn;
+                       goto out_cache_error;
                }
                status = nfserr_wrong_cred;
                if (!nfsd4_mach_creds_match(unconf, rqstp))
-                       goto out_free_conn;
-               cs_slot = &unconf->cl_cs_slot;
-               status = check_slot_seqid(cr_ses->seqid, cs_slot->sl_seqid, 0);
-               if (status) {
-                       /* an unconfirmed replay returns misordered */
-                       status = nfserr_seq_misordered;
-                       goto out_free_conn;
-               }
+                       goto out_cache_error;
                old = find_confirmed_client_by_name(&unconf->cl_name, nn);
                if (old) {
                        status = mark_client_expired_locked(old);
                        if (status) {
                                old = NULL;
-                               goto out_free_conn;
+                               goto out_cache_error;
                        }
                        trace_nfsd_clid_replaced(&old->cl_clientid);
                }
                move_to_confirmed(unconf);
                conf = unconf;
-       } else {
-               status = nfserr_stale_clientid;
-               goto out_free_conn;
        }
        status = nfs_ok;
        /* Persistent sessions are not supported */
@@ -3817,8 +3821,6 @@ nfsd4_create_session(struct svc_rqst *rqstp,
 
        memcpy(cr_ses->sessionid.data, new->se_sessionid.data,
               NFS4_MAX_SESSIONID_LEN);
-       cs_slot->sl_seqid++;
-       cr_ses->seqid = cs_slot->sl_seqid;
 
        /* cache solo and embedded create sessions under the client_lock */
        nfsd4_cache_create_session(cr_ses, cs_slot, status);
@@ -3831,6 +3833,9 @@ nfsd4_create_session(struct svc_rqst *rqstp,
        if (old)
                expire_client(old);
        return status;
+
+out_cache_error:
+       nfsd4_cache_create_session(cr_ses, cs_slot, status);
 out_free_conn:
        spin_unlock(&nn->client_lock);
        free_conn(conn);