> On Jan 27, 2022, at 2:42 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > On Thu, Jan 27, 2022 at 03:51:54PM +0000, Chuck Lever III wrote: >> Hi Dai- >> >>> On Jan 26, 2022, at 4:13 PM, Dai Ngo <dai.ngo@xxxxxxxxxx> wrote: >>> >>> From RFC 7530 Section 16.34.5: >>> >>> o The server has not recorded an unconfirmed { v, x, c, *, * } and >>> has recorded a confirmed { v, x, c, *, s }. If the principals of >>> the record and of SETCLIENTID_CONFIRM do not match, the server >>> returns NFS4ERR_CLID_INUSE without removing any relevant leased >>> client state, and without changing recorded callback and >>> callback_ident values for client { x }. >>> >>> The current code intents to do what the spec describes above but >>> it forgot to set 'old' to NULL resulting to the confirmed client >>> to be expired. >>> >>> Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx> >> >> On it's face, this seems like the correct thing to do. >> >> I believe the issue was introduced in commit 2b63482185e6 ("nfsd: >> fix clid_inuse on mount with security change") in 2015. I can >> add a Fixes: tag and apply this for 5.17-rc. > > Looks right to me too--thanks, Dai. May I add a Reviewed-by: Bruce ? > > --b. > >>> --- >>> fs/nfsd/nfs4state.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index 72900b89cf84..32063733443d 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -4130,8 +4130,10 @@ nfsd4_setclientid_confirm(struct svc_rqst *rqstp, >>> status = nfserr_clid_inuse; >>> if (client_has_state(old) >>> && !same_creds(&unconf->cl_cred, >>> - &old->cl_cred)) >>> + &old->cl_cred)) { >>> + old = NULL; >>> goto out; >>> + } >>> status = mark_client_expired_locked(old); >>> if (status) { >>> old = NULL; >>> -- >>> 2.9.5 >>> >> >> -- >> Chuck Lever -- Chuck Lever