Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Oct 2023 19:36:49 GMT
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: f300335d9aeb - main - nfsd: Fix NFSv4.1/4.2 Claim_Deleg_Cur_FH
Message-ID:  <202310191936.39JJaniG094346@gitrepo.freebsd.org>

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

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

commit f300335d9aebf2e99862bf783978bd44ede23550
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2023-10-19 19:35:35 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2023-10-19 19:35:35 +0000

    nfsd: Fix NFSv4.1/4.2 Claim_Deleg_Cur_FH
    
    When I implemented a test patch using Open Claim_Deleg_Cur_FH
    I discovered that the NFSv4.1/4.2 server was broken for this
    Open option.  Fortunately it is never used by the FreeBSD
    client and never used by other clients unless delegations
    are enabled. (The FreeBSD NFSv4 server does not have delegations
    enabled by default.)
    
    Claim_Deleg_Cur_FH was broken because the code mistakenly
    assumed a stateID argument, which is not the case.
    This patch fixes the bug by changing the XDR parser to not
    expect a stateID and to fill most of the stateID in from the
    clientID. The clientID is the first two elements of the "other"
    array for the stateID and is sufficient to identify which
    client the delegation is issued to.  Since there is only one
    delegation issued to a client per file, this is sufficient to
    locate the correct delegation.
    
    If you are running non-FreeBSD NFSv4.1/4.2 mounts against the
    FreeBSD server, you need this patch if you have delegations enabled.
    
    PR:     274574
    MFC after:      2 weeks
---
 sys/fs/nfsserver/nfs_nfsdserv.c  | 10 ++++++++--
 sys/fs/nfsserver/nfs_nfsdstate.c | 16 ++++++++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/sys/fs/nfsserver/nfs_nfsdserv.c b/sys/fs/nfsserver/nfs_nfsdserv.c
index 7d3ebc683d16..3daee65ab83a 100644
--- a/sys/fs/nfsserver/nfs_nfsdserv.c
+++ b/sys/fs/nfsserver/nfs_nfsdserv.c
@@ -3000,12 +3000,18 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int isdgram,
 	 */
 	NFSM_DISSECT(tl, u_int32_t *, NFSX_UNSIGNED);
 	claim = fxdr_unsigned(int, *tl);
-	if (claim == NFSV4OPEN_CLAIMDELEGATECUR || claim ==
-	    NFSV4OPEN_CLAIMDELEGATECURFH) {
+	if (claim == NFSV4OPEN_CLAIMDELEGATECUR) {
 		NFSM_DISSECT(tl, u_int32_t *, NFSX_STATEID);
 		stateid.seqid = fxdr_unsigned(u_int32_t, *tl++);
 		NFSBCOPY((caddr_t)tl,(caddr_t)stateid.other,NFSX_STATEIDOTHER);
 		stp->ls_flags |= NFSLCK_DELEGCUR;
+	} else if (claim == NFSV4OPEN_CLAIMDELEGATECURFH) {
+		/* Fill in most of the stateid from the clientid. */
+		stateid.seqid = 0;
+		stateid.other[0] = clientid.lval[0];
+		stateid.other[1] = clientid.lval[1];
+		stateid.other[2] = 0;
+		stp->ls_flags |= NFSLCK_DELEGCUR;
 	} else if (claim == NFSV4OPEN_CLAIMDELEGATEPREV || claim ==
 	    NFSV4OPEN_CLAIMDELEGATEPREVFH) {
 		stp->ls_flags |= NFSLCK_DELEGPREV;
diff --git a/sys/fs/nfsserver/nfs_nfsdstate.c b/sys/fs/nfsserver/nfs_nfsdstate.c
index c73840277022..da57ebde7a52 100644
--- a/sys/fs/nfsserver/nfs_nfsdstate.c
+++ b/sys/fs/nfsserver/nfs_nfsdstate.c
@@ -2568,6 +2568,10 @@ tryagain:
 	    /*
 	     * For Delegate_Cur, search for the matching Delegation,
 	     * which indicates no conflict.
+	     * For NFSv4.1/4.2 Claim_Deleg_Cur_FH only provides
+	     * the clientid, which is the first two "other" elements
+	     * for the stateid.  This should be sufficient, since there
+	     * is only one delegation per client and file.
 	     * An old delegation should have been recovered by the
 	     * client doing a Claim_DELEGATE_Prev, so I won't let
 	     * it match and return NFSERR_EXPIRED. Should I let it
@@ -2578,8 +2582,8 @@ tryagain:
 		    (((nd->nd_flag & ND_NFSV41) != 0 &&
 		    stateidp->seqid == 0) ||
 		    stateidp->seqid == stp->ls_stateid.seqid) &&
-		    !NFSBCMP(stateidp->other, stp->ls_stateid.other,
-			  NFSX_STATEIDOTHER))
+		    stateidp->other[0] == stp->ls_stateid.other[0] &&
+		    stateidp->other[1] == stp->ls_stateid.other[1])
 			break;
 	    }
 	    if (stp == LIST_END(&lfp->lf_deleg) ||
@@ -2830,6 +2834,10 @@ tryagain:
 	    /*
 	     * For Delegate_Cur, search for the matching Delegation,
 	     * which indicates no conflict.
+	     * For NFSv4.1/4.2 Claim_Deleg_Cur_FH only provides
+	     * the clientid, which is the first two "other" elements
+	     * for the stateid.  This should be sufficient, since there
+	     * is only one delegation per client and file.
 	     * An old delegation should have been recovered by the
 	     * client doing a Claim_DELEGATE_Prev, so I won't let
 	     * it match and return NFSERR_EXPIRED. Should I let it
@@ -2840,8 +2848,8 @@ tryagain:
 		    (((nd->nd_flag & ND_NFSV41) != 0 &&
 		    stateidp->seqid == 0) ||
 		    stateidp->seqid == stp->ls_stateid.seqid) &&
-		    !NFSBCMP(stateidp->other, stp->ls_stateid.other,
-			NFSX_STATEIDOTHER))
+		    stateidp->other[0] == stp->ls_stateid.other[0] &&
+		    stateidp->other[1] == stp->ls_stateid.other[1])
 			break;
 	    }
 	    if (stp == LIST_END(&lfp->lf_deleg) ||



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