> 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
