git: f383f4ad30a6 - stable/12 - nfsd: Fix NFSv4.1/4.2 Claim_Deleg_Cur_FH
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 02 Nov 2023 23:45:40 UTC
The branch stable/12 has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=f383f4ad30a61fd0cd5104bb5c730aff139b993d commit f383f4ad30a61fd0cd5104bb5c730aff139b993d Author: Rick Macklem <rmacklem@FreeBSD.org> AuthorDate: 2023-10-19 19:35:35 +0000 Commit: Rick Macklem <rmacklem@FreeBSD.org> CommitDate: 2023-11-02 23:44:27 +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 (cherry picked from commit f300335d9aebf2e99862bf783978bd44ede23550) --- 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 e283e8611944..e5eced9c8a69 100644 --- a/sys/fs/nfsserver/nfs_nfsdserv.c +++ b/sys/fs/nfsserver/nfs_nfsdserv.c @@ -2913,12 +2913,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 cb4597ec1575..f5f498f61b6b 100644 --- a/sys/fs/nfsserver/nfs_nfsdstate.c +++ b/sys/fs/nfsserver/nfs_nfsdstate.c @@ -2529,6 +2529,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 @@ -2539,8 +2543,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) || @@ -2791,6 +2795,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 @@ -2801,8 +2809,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) ||