Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 Jul 2022 20:26:36 GMT
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: db2eff1cff8c - stable/13 - nfsd: Fix CreateSession for an established ClientID
Message-ID:  <202207282026.26SKQah9006957@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by rmacklem:

URL: https://cgit.FreeBSD.org/src/commit/?id=db2eff1cff8c374f04030bb116c07e3112ecc516

commit db2eff1cff8c374f04030bb116c07e3112ecc516
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2022-07-13 23:28:56 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2022-07-28 20:20:33 +0000

    nfsd: Fix CreateSession for an established ClientID
    
    I mis-read the RFC w.r.t. handling of the sequenceid
    when a CreateSession is done after the initial one
    that confirms the ClientID.  Fortunately this does
    not affect most extant NFSv4.1/4.2 clients, since
    they only acquire a single session for TCP for a
    ClientID (Solaris might be an exception?).
    
    This patch fixes the server to handle this case,
    where the RFC requires the sequenceid be incremented
    for each CreateSession and is required to reply to
    a retried CreateSession with a cached reply.
    It adds a field to nfsclient called lc_prevsess,
    which caches the sessionid, which is the only field
    in a CreateSession reply that will change for a
    retry, to implement this reply cache.
    
    The recent commits up to d4a11b3e3bdd that mark
    session slots bad when "intr" and/or "soft" mounts
    are used by the client needs this server patch.
    Without this patch, the client will do a full
    recovery, including a new ClientID, losing all
    byte range locks.  However, prior to the recent
    client commits, the client would hang when all
    session slots were bad, so even without this
    patch it is not a regression.
    
    PR: 260011
    (cherry picked from commit 088ba4356a114416b1d68bf5ae3b3e0accf6d0df)
---
 sys/fs/nfs/nfsrvstate.h          |  1 +
 sys/fs/nfsserver/nfs_nfsdstate.c | 63 ++++++++++++++++++++++++++--------------
 2 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/sys/fs/nfs/nfsrvstate.h b/sys/fs/nfs/nfsrvstate.h
index 9eebeece9727..4c171e8b3f50 100644
--- a/sys/fs/nfs/nfsrvstate.h
+++ b/sys/fs/nfs/nfsrvstate.h
@@ -99,6 +99,7 @@ struct nfsclient {
 	struct nfsstatehead lc_deleg;		/* Delegations */
 	struct nfsstatehead lc_olddeleg;	/* and old delegations */
 	struct nfssessionhead lc_session;	/* List of NFSv4.1 sessions */
+	uint64_t	lc_prevsess;		/* CreateSession cache */
 	time_t		lc_expiry;		/* Expiry time (sec) */
 	time_t		lc_delegtime;		/* Old deleg expiry (sec) */
 	nfsquad_t	lc_clientid;		/* 64 bit clientid */
diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
index 842d3c75f678..6b2a6e56795d 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -336,10 +336,10 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
 		 * Add it after assigning a client id to it.
 		 */
 		new_clp->lc_flags |= LCL_NEEDSCONFIRM;
-		if ((nd->nd_flag & ND_NFSV41) != 0)
-			new_clp->lc_confirm.lval[0] = confirmp->lval[0] =
-			    ++confirm_index;
-		else
+		if ((nd->nd_flag & ND_NFSV41) != 0) {
+			confirmp->lval[0] = ++confirm_index;
+			new_clp->lc_confirm.lval[0] = confirmp->lval[0] - 1;
+		} else
 			confirmp->qval = new_clp->lc_confirm.qval =
 			    ++confirm_index;
 		clientidp->lval[0] = new_clp->lc_clientid.lval[0] =
@@ -348,6 +348,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
 		    nfsrv_nextclientindex();
 		new_clp->lc_stateindex = 0;
 		new_clp->lc_statemaxindex = 0;
+		new_clp->lc_prevsess = 0;
 		new_clp->lc_cbref = 0;
 		new_clp->lc_expiry = nfsrv_leaseexpiry();
 		LIST_INIT(&new_clp->lc_open);
@@ -449,10 +450,10 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
 		}
 
 		new_clp->lc_flags |= LCL_NEEDSCONFIRM;
-		if ((nd->nd_flag & ND_NFSV41) != 0)
-			new_clp->lc_confirm.lval[0] = confirmp->lval[0] =
-			    ++confirm_index;
-		else
+		if ((nd->nd_flag & ND_NFSV41) != 0) {
+			confirmp->lval[0] = ++confirm_index;
+			new_clp->lc_confirm.lval[0] = confirmp->lval[0] - 1;
+		} else
 			confirmp->qval = new_clp->lc_confirm.qval =
 			    ++confirm_index;
 		clientidp->lval[0] = new_clp->lc_clientid.lval[0] =
@@ -461,6 +462,7 @@ nfsrv_setclient(struct nfsrv_descript *nd, struct nfsclient **new_clpp,
 		    nfsrv_nextclientindex();
 		new_clp->lc_stateindex = 0;
 		new_clp->lc_statemaxindex = 0;
+		new_clp->lc_prevsess = 0;
 		new_clp->lc_cbref = 0;
 		new_clp->lc_expiry = nfsrv_leaseexpiry();
 
@@ -596,6 +598,7 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
 	struct nfssessionhash *shp;
 	struct nfsdsession *sep;
 	uint64_t sessid[2];
+	bool sess_replay;
 	static uint64_t next_sess = 0;
 
 	if (clpp)
@@ -676,13 +679,29 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
 	 * Perform any operations specified by the opflags.
 	 */
 	if (opflags & CLOPS_CONFIRM) {
-		if ((nd->nd_flag & ND_NFSV41) != 0 &&
-		     clp->lc_confirm.lval[0] != confirm.lval[0])
+		sess_replay = false;
+		if ((nd->nd_flag & ND_NFSV41) != 0) {
+		    /*
+		     * For the case where lc_confirm.lval[0] == confirm.lval[0],
+		     * use the new session, but with the previous sessionid.
+		     * This is not exactly what the RFC describes, but should
+		     * result in the same reply as the previous CreateSession.
+		     */
+		    if (clp->lc_confirm.lval[0] + 1 == confirm.lval[0]) {
+			clp->lc_confirm.lval[0] = confirm.lval[0];
+			clp->lc_prevsess = sessid[0];
+		    } else if (clp->lc_confirm.lval[0] == confirm.lval[0]) {
+			if (clp->lc_prevsess == 0)
+			    error = NFSERR_SEQMISORDERED;
+			else
+			    sessid[0] = clp->lc_prevsess;
+			sess_replay = true;
+		    } else
 			error = NFSERR_SEQMISORDERED;
-		else if ((nd->nd_flag & ND_NFSV41) == 0 &&
+		} else if ((nd->nd_flag & ND_NFSV41) == 0 &&
 		     clp->lc_confirm.qval != confirm.qval)
 			error = NFSERR_STALECLIENTID;
-		else if (nfsrv_notsamecredname(nd, clp))
+		if (error == 0 && nfsrv_notsamecredname(nd, clp))
 			error = NFSERR_CLIDINUSE;
 
 		if (!error) {
@@ -716,7 +735,7 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
 		    if (nsep != NULL) {
 			/* Hold a reference on the xprt for a backchannel. */
 			if ((nsep->sess_crflags & NFSV4CRSESS_CONNBACKCHAN)
-			    != 0) {
+			    != 0 && !sess_replay) {
 			    if (clp->lc_req.nr_client == NULL)
 				clp->lc_req.nr_client = (struct __rpc_client *)
 				    clnt_bck_create(nd->nd_xprt->xp_socket,
@@ -735,14 +754,16 @@ nfsrv_getclient(nfsquad_t clientid, int opflags, struct nfsclient **clpp,
 			    NFSX_V4SESSIONID);
 			NFSBCOPY(sessid, nsep->sess_cbsess.nfsess_sessionid,
 			    NFSX_V4SESSIONID);
-			shp = NFSSESSIONHASH(nsep->sess_sessionid);
-			NFSLOCKSTATE();
-			NFSLOCKSESSION(shp);
-			LIST_INSERT_HEAD(&shp->list, nsep, sess_hash);
-			LIST_INSERT_HEAD(&clp->lc_session, nsep, sess_list);
-			nsep->sess_clp = clp;
-			NFSUNLOCKSESSION(shp);
-			NFSUNLOCKSTATE();
+			if (!sess_replay) {
+			    shp = NFSSESSIONHASH(nsep->sess_sessionid);
+			    NFSLOCKSTATE();
+			    NFSLOCKSESSION(shp);
+			    LIST_INSERT_HEAD(&shp->list, nsep, sess_hash);
+			    LIST_INSERT_HEAD(&clp->lc_session, nsep, sess_list);
+			    nsep->sess_clp = clp;
+			    NFSUNLOCKSESSION(shp);
+			    NFSUNLOCKSTATE();
+			}
 		    }
 		}
 	} else if (clp->lc_flags & LCL_NEEDSCONFIRM) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202207282026.26SKQah9006957>