* [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client.
@ 2022-01-26 21:13 Dai Ngo
2022-01-27 15:51 ` Chuck Lever III
0 siblings, 1 reply; 5+ messages in thread
From: Dai Ngo @ 2022-01-26 21:13 UTC (permalink / raw)
To: chuck.lever; +Cc: linux-nfs
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@oracle.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client.
2022-01-26 21:13 [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client Dai Ngo
@ 2022-01-27 15:51 ` Chuck Lever III
2022-01-27 19:42 ` J. Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Chuck Lever III @ 2022-01-27 15:51 UTC (permalink / raw)
To: Dai Ngo; +Cc: Linux NFS Mailing List
Hi Dai-
> On Jan 26, 2022, at 4:13 PM, Dai Ngo <dai.ngo@oracle.com> 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@oracle.com>
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.
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client.
2022-01-27 15:51 ` Chuck Lever III
@ 2022-01-27 19:42 ` J. Bruce Fields
2022-01-28 14:02 ` Chuck Lever III
0 siblings, 1 reply; 5+ messages in thread
From: J. Bruce Fields @ 2022-01-27 19:42 UTC (permalink / raw)
To: Chuck Lever III; +Cc: Dai Ngo, Linux NFS Mailing List
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@oracle.com> 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@oracle.com>
>
> 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.
--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
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client.
2022-01-27 19:42 ` J. Bruce Fields
@ 2022-01-28 14:02 ` Chuck Lever III
2022-01-28 15:04 ` Bruce Fields
0 siblings, 1 reply; 5+ messages in thread
From: Chuck Lever III @ 2022-01-28 14:02 UTC (permalink / raw)
To: Bruce Fields; +Cc: Dai Ngo, Linux NFS Mailing List
> On Jan 27, 2022, at 2:42 PM, J. Bruce Fields <bfields@fieldses.org> 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@oracle.com> 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@oracle.com>
>>
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client.
2022-01-28 14:02 ` Chuck Lever III
@ 2022-01-28 15:04 ` Bruce Fields
0 siblings, 0 replies; 5+ messages in thread
From: Bruce Fields @ 2022-01-28 15:04 UTC (permalink / raw)
To: Chuck Lever III; +Cc: Dai Ngo, Linux NFS Mailing List
On Fri, Jan 28, 2022 at 02:02:57PM +0000, Chuck Lever III wrote:
>
>
> > On Jan 27, 2022, at 2:42 PM, J. Bruce Fields <bfields@fieldses.org> 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@oracle.com> 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@oracle.com>
> >>
> >> 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 ?
Sure.--b.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-28 15:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-26 21:13 [PATCH 1/1] nfsd: nfsd4_setclientid_confirm mistakenly expires confirmed client Dai Ngo
2022-01-27 15:51 ` Chuck Lever III
2022-01-27 19:42 ` J. Bruce Fields
2022-01-28 14:02 ` Chuck Lever III
2022-01-28 15:04 ` Bruce Fields
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.