diff mbox

v3.5 nfsd4 regression; utime sometimes takes 40+ seconds to return

Message ID 20120816210916.GA6108@fieldses.org (mailing list archive)
State New, archived
Headers show

Commit Message

J. Bruce Fields Aug. 16, 2012, 9:09 p.m. UTC 
On Thu, Aug 16, 2012 at 04:42:08PM -0400, J. Bruce Fields wrote:
> Hm, weird.  In the good case the cb_recall's done with auth_unix, in the
> bad case with auth_null.  OK, that should be enough to go on....
> 
> Thanks for digging into this!

This should fix it--could you confirm?

--b.

commit e950bebdac1f17121f972728489cdba43734d56d
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Thu Aug 16 17:01:21 2012 -0400

    nfsd4: fix security flavor of NFSv4.0 callback
    
    Commit d5497fc693a446ce9100fcf4117c3f795ddfd0d2 "nfsd4: move rq_flavor
    into svc_cred" forgot to remove cl_flavor from the client, leaving two
    places (cl_flavor and cl_cred.cr_flavor) for the flavor to be stored.
    After that patch, the latter was the one that was updated, but the
    former was the one that the callback used.
    
    Symptoms were a long delay on utime().  This is because the utime()
    generated a setattr which recalled a delegation, but the cb_recall was
    ignored by the client because it had the wrong security flavor.
    
    Cc: stable@vger.kernel.org
    Reported-by: Jamie Heilman <jamie@audible.transient.net>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jamie Heilman Aug. 16, 2012, 9:34 p.m. UTC | #1 
J. Bruce Fields wrote:
> On Thu, Aug 16, 2012 at 04:42:08PM -0400, J. Bruce Fields wrote:
> > Hm, weird.  In the good case the cb_recall's done with auth_unix, in the
> > bad case with auth_null.  OK, that should be enough to go on....
> > 
> > Thanks for digging into this!
> 
> This should fix it--could you confirm?

Yep, looks like that fixes it.

> commit e950bebdac1f17121f972728489cdba43734d56d
> Author: J. Bruce Fields <bfields@redhat.com>
> Date:   Thu Aug 16 17:01:21 2012 -0400
> 
>     nfsd4: fix security flavor of NFSv4.0 callback
>     
>     Commit d5497fc693a446ce9100fcf4117c3f795ddfd0d2 "nfsd4: move rq_flavor
>     into svc_cred" forgot to remove cl_flavor from the client, leaving two
>     places (cl_flavor and cl_cred.cr_flavor) for the flavor to be stored.
>     After that patch, the latter was the one that was updated, but the
>     former was the one that the callback used.
>     
>     Symptoms were a long delay on utime().  This is because the utime()
>     generated a setattr which recalled a delegation, but the cb_recall was
>     ignored by the client because it had the wrong security flavor.
>     
>     Cc: stable@vger.kernel.org
>     Reported-by: Jamie Heilman <jamie@audible.transient.net>
>     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index cbaf4f8..4c7bd35 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -651,12 +651,12 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
>  
>  	if (clp->cl_minorversion == 0) {
>  		if (!clp->cl_cred.cr_principal &&
> -				(clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
> +				(clp->cl_cred.cr_flavor >= RPC_AUTH_GSS_KRB5))
>  			return -EINVAL;
>  		args.client_name = clp->cl_cred.cr_principal;
>  		args.prognumber	= conn->cb_prog,
>  		args.protocol = XPRT_TRANSPORT_TCP;
> -		args.authflavor = clp->cl_flavor;
> +		args.authflavor = clp->cl_cred.cr_flavor;
>  		clp->cl_cb_ident = conn->cb_ident;
>  	} else {
>  		if (!conn->cb_xprt)
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 167d7d8..9db0bb5 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -231,7 +231,6 @@ struct nfs4_client {
>  	nfs4_verifier		cl_verifier; 	/* generated by client */
>  	time_t                  cl_time;        /* time of last lease renewal */
>  	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
> -	u32			cl_flavor;	/* setclientid pseudoflavor */
>  	struct svc_cred		cl_cred; 	/* setclientid principal */
>  	clientid_t		cl_clientid;	/* generated by server */
>  	nfs4_verifier		cl_confirm;	/* generated by server */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
J. Bruce Fields Aug. 17, 2012, 1:08 p.m. UTC | #2 
On Thu, Aug 16, 2012 at 09:34:24PM +0000, Jamie Heilman wrote:
> J. Bruce Fields wrote:
> > On Thu, Aug 16, 2012 at 04:42:08PM -0400, J. Bruce Fields wrote:
> > > Hm, weird.  In the good case the cb_recall's done with auth_unix, in the
> > > bad case with auth_null.  OK, that should be enough to go on....
> > > 
> > > Thanks for digging into this!
> > 
> > This should fix it--could you confirm?
> 
> Yep, looks like that fixes it.

Thanks!

I'll queue this up for 3.6 and stable.

--b.

> 
> > commit e950bebdac1f17121f972728489cdba43734d56d
> > Author: J. Bruce Fields <bfields@redhat.com>
> > Date:   Thu Aug 16 17:01:21 2012 -0400
> > 
> >     nfsd4: fix security flavor of NFSv4.0 callback
> >     
> >     Commit d5497fc693a446ce9100fcf4117c3f795ddfd0d2 "nfsd4: move rq_flavor
> >     into svc_cred" forgot to remove cl_flavor from the client, leaving two
> >     places (cl_flavor and cl_cred.cr_flavor) for the flavor to be stored.
> >     After that patch, the latter was the one that was updated, but the
> >     former was the one that the callback used.
> >     
> >     Symptoms were a long delay on utime().  This is because the utime()
> >     generated a setattr which recalled a delegation, but the cb_recall was
> >     ignored by the client because it had the wrong security flavor.
> >     
> >     Cc: stable@vger.kernel.org
> >     Reported-by: Jamie Heilman <jamie@audible.transient.net>
> >     Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> > 
> > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> > index cbaf4f8..4c7bd35 100644
> > --- a/fs/nfsd/nfs4callback.c
> > +++ b/fs/nfsd/nfs4callback.c
> > @@ -651,12 +651,12 @@ static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
> >  
> >  	if (clp->cl_minorversion == 0) {
> >  		if (!clp->cl_cred.cr_principal &&
> > -				(clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
> > +				(clp->cl_cred.cr_flavor >= RPC_AUTH_GSS_KRB5))
> >  			return -EINVAL;
> >  		args.client_name = clp->cl_cred.cr_principal;
> >  		args.prognumber	= conn->cb_prog,
> >  		args.protocol = XPRT_TRANSPORT_TCP;
> > -		args.authflavor = clp->cl_flavor;
> > +		args.authflavor = clp->cl_cred.cr_flavor;
> >  		clp->cl_cb_ident = conn->cb_ident;
> >  	} else {
> >  		if (!conn->cb_xprt)
> > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > index 167d7d8..9db0bb5 100644
> > --- a/fs/nfsd/state.h
> > +++ b/fs/nfsd/state.h
> > @@ -231,7 +231,6 @@ struct nfs4_client {
> >  	nfs4_verifier		cl_verifier; 	/* generated by client */
> >  	time_t                  cl_time;        /* time of last lease renewal */
> >  	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
> > -	u32			cl_flavor;	/* setclientid pseudoflavor */
> >  	struct svc_cred		cl_cred; 	/* setclientid principal */
> >  	clientid_t		cl_clientid;	/* generated by server */
> >  	nfs4_verifier		cl_confirm;	/* generated by server */
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> 
> -- 
> Jamie Heilman                     http://audible.transient.net/~jamie/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index cbaf4f8..4c7bd35 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -651,12 +651,12 @@  static int setup_callback_client(struct nfs4_client *clp, struct nfs4_cb_conn *c
 
 	if (clp->cl_minorversion == 0) {
 		if (!clp->cl_cred.cr_principal &&
-				(clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
+				(clp->cl_cred.cr_flavor >= RPC_AUTH_GSS_KRB5))
 			return -EINVAL;
 		args.client_name = clp->cl_cred.cr_principal;
 		args.prognumber	= conn->cb_prog,
 		args.protocol = XPRT_TRANSPORT_TCP;
-		args.authflavor = clp->cl_flavor;
+		args.authflavor = clp->cl_cred.cr_flavor;
 		clp->cl_cb_ident = conn->cb_ident;
 	} else {
 		if (!conn->cb_xprt)
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 167d7d8..9db0bb5 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -231,7 +231,6 @@  struct nfs4_client {
 	nfs4_verifier		cl_verifier; 	/* generated by client */
 	time_t                  cl_time;        /* time of last lease renewal */
 	struct sockaddr_storage	cl_addr; 	/* client ipaddress */
-	u32			cl_flavor;	/* setclientid pseudoflavor */
 	struct svc_cred		cl_cred; 	/* setclientid principal */
 	clientid_t		cl_clientid;	/* generated by server */
 	nfs4_verifier		cl_confirm;	/* generated by server */