* [PATCH 0/3] Use root user credential for lease management
@ 2013-06-07 19:44 Chuck Lever
2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chuck Lever @ 2013-06-07 19:44 UTC (permalink / raw)
To: linux-nfs
Hi-
This series applies to post 3.10-rc4 kernels. The idea is to make
newer kernels behave like 3.6 and previous when it comes to mounting
sec=krb5 when the client has no keytab available.
The third patch is similar to the 3.7 patch I posted a few days ago,
but is adjusted slightly to apply after the "Use krb5i for lease
management" patch that appears in 3.10-rc.
---
Chuck Lever (3):
NFS: Never use user credentials for lease renewal
NFS: Partially revert commit 845cbceb
NFS: Use root's credential for lease management when keytab is missing
fs/nfs/nfs4proc.c | 25 ++++++++++++++++---------
fs/nfs/nfs4state.c | 28 ++++++++++++++++++++--------
2 files changed, 36 insertions(+), 17 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] NFS: Never use user credentials for lease renewal 2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever @ 2013-06-07 19:44 ` Chuck Lever 2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever 2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever 2 siblings, 0 replies; 6+ messages in thread From: Chuck Lever @ 2013-06-07 19:44 UTC (permalink / raw) To: linux-nfs Don't try to use a non-UID-0 user credential for lease management, as that credential can change out from under us. The server will block NFSv4 lease recovery with NFS4ERR_CLID_INUSE. Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfs/nfs4proc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index d7ba561..5ba38b3 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -6919,7 +6919,7 @@ static const struct nfs4_state_recovery_ops nfs40_reboot_recovery_ops = { .recover_open = nfs4_open_reclaim, .recover_lock = nfs4_lock_reclaim, .establish_clid = nfs4_init_clientid, - .get_clid_cred = nfs4_get_setclientid_cred, + .get_clid_cred = nfs4_get_exchange_id_cred, .detect_trunking = nfs40_discover_server_trunking, }; @@ -6942,7 +6942,7 @@ static const struct nfs4_state_recovery_ops nfs40_nograce_recovery_ops = { .recover_open = nfs4_open_expired, .recover_lock = nfs4_lock_expired, .establish_clid = nfs4_init_clientid, - .get_clid_cred = nfs4_get_setclientid_cred, + .get_clid_cred = nfs4_get_exchange_id_cred, }; #if defined(CONFIG_NFS_V4_1) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] NFS: Partially revert commit 845cbceb 2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever 2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever @ 2013-06-07 19:45 ` Chuck Lever 2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever 2 siblings, 0 replies; 6+ messages in thread From: Chuck Lever @ 2013-06-07 19:45 UTC (permalink / raw) To: linux-nfs I'm about to add logic that invokes nfs4_clear_machine_cred(), but we removed it in 3.10. This is a separate commit to make it easier to backport the next patch to a kernel that may still have nfs4_clear_machine_cred(). Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfs/nfs4state.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 1fab140..4949ce5 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -154,6 +154,18 @@ struct rpc_cred *nfs4_get_machine_cred_locked(struct nfs_client *clp) return cred; } +static void nfs4_clear_machine_cred(struct nfs_client *clp) +{ + struct rpc_cred *cred; + + spin_lock(&clp->cl_lock); + cred = clp->cl_machine_cred; + clp->cl_machine_cred = NULL; + spin_unlock(&clp->cl_lock); + if (cred != NULL) + put_rpccred(cred); +} + static struct rpc_cred * nfs4_get_renew_cred_server_locked(struct nfs_server *server) { ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing 2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever 2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever 2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever @ 2013-06-07 19:45 ` Chuck Lever 2013-06-07 19:52 ` Myklebust, Trond 2 siblings, 1 reply; 6+ messages in thread From: Chuck Lever @ 2013-06-07 19:45 UTC (permalink / raw) To: linux-nfs Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting" Fri Sep 14 17:24:32 2012 introduced Uniform Client String support, which forces our NFS client to establish a client ID immediately during a mount operation rather than waiting until a user wants to open a file. Normally machine credentials (eg. from a keytab) are used to perform a mount operation that is protected by Kerberos. Before 05fc350, SETCLIENTID used a machine credential, or fell back to a regular user's credential if no keytab is available. On clients that don't have a keytab, performing SETCLIENTID early means there's no user credential to fall back on, since no regular user has kinit'd yet. 05f4c350 seems to have broken the ability to mount with sec=krb5 on clients that don't have a keytab in kernels 3.7 - 3.9. To address this regression, commit 4edaa308 (NFS: Use "krb5i" to establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013, was merged in 3.10. This commit forces the NFS client to fall back to AUTH_SYS for lease management operations if no keytab is available. Neil Brown noticed that, since root is required to kinit to do a sec=krb5 mount when a client doesn't have a keytab, we can try to use root's Kerberos credential before AUTH_SYS. Now, when determining a principal and flavor to use for lease management, the NFS client tries in this order: 1. Flavor: AUTH_GSS, krb5i Principal: service principal (via keytab) 2. Flavor: AUTH_GSS, krb5i Principal: user principal established for UID 0 (via kinit) 3. Flavor: AUTH_SYS Principal: UID 0 / GID 0 Signed-off-by: Chuck Lever <chuck.lever@oracle.com> --- fs/nfs/nfs4proc.c | 21 ++++++++++++++------- fs/nfs/nfs4state.c | 16 ++++++++-------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 5ba38b3..4d76fae 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, dprintk("NFS call setclientid auth=%s, '%.*s'\n", clp->cl_rpcclient->cl_auth->au_ops->au_name, setclientid.sc_name_len, setclientid.sc_name); - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); dprintk("NFS reply setclientid: %d\n", status); return status; } @@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp, dprintk("NFS call setclientid_confirm auth=%s, (client ID %llx)\n", clp->cl_rpcclient->cl_auth->au_ops->au_name, clp->cl_clientid); - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); dprintk("NFS reply setclientid_confirm: %d\n", status); return status; } @@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred goto out; } - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); if (status == 0) { if (memcmp(res.session->sess_id.data, clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) { @@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred) goto out_server_scope; } - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); if (status == 0) status = nfs4_check_cl_exchange_flags(res.flags); @@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp, }; int status; - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); if (status) dprintk("NFS: Got error %d from the server %s on " "DESTROY_CLIENTID.", status, clp->cl_hostname); @@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp, nfs4_init_channel_attrs(&args); args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN); - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); if (!status) { /* Verify the session's negotiated channel_attrs values */ @@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session, if (session->clp->cl_cons_state != NFS_CS_READY) return status; - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); if (status) dprintk("NFS: Got error %d from the server on DESTROY_SESSION. " diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c index 4949ce5..d8068ab 100644 --- a/fs/nfs/nfs4state.c +++ b/fs/nfs/nfs4state.c @@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp) int status; cred = ops->get_clid_cred(clp); - if (cred == NULL) - return -ENOENT; status = ops->establish_clid(clp, cred); - put_rpccred(cred); + if (cred) + put_rpccred(cred); if (status != 0) return status; pnfs_destroy_all_layouts(clp); @@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp, again: status = -ENOENT; cred = ops->get_clid_cred(clp); - if (cred == NULL) - goto out_unlock; - status = ops->detect_trunking(clp, result, cred); - put_rpccred(cred); + if (cred) + put_rpccred(cred); switch (status) { case 0: break; @@ -1902,6 +1899,10 @@ again: __func__, status); goto again; case -EACCES: + if (clp->cl_machine_cred != NULL) { + nfs4_clear_machine_cred(clp); + goto again; + } if (i++) break; case -NFS4ERR_CLID_INUSE: @@ -1935,7 +1936,6 @@ again: status = -EIO; } -out_unlock: mutex_unlock(&nfs_clid_init_mutex); dprintk("NFS: %s: status = %d\n", __func__, status); return status; ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing 2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever @ 2013-06-07 19:52 ` Myklebust, Trond 2013-06-07 20:04 ` Chuck Lever 0 siblings, 1 reply; 6+ messages in thread From: Myklebust, Trond @ 2013-06-07 19:52 UTC (permalink / raw) To: Chuck Lever; +Cc: linux-nfs@vger.kernel.org On Fri, 2013-06-07 at 15:45 -0400, Chuck Lever wrote: > Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting" > Fri Sep 14 17:24:32 2012 introduced Uniform Client String support, > which forces our NFS client to establish a client ID immediately > during a mount operation rather than waiting until a user wants to > open a file. > > Normally machine credentials (eg. from a keytab) are used to perform > a mount operation that is protected by Kerberos. Before 05fc350, > SETCLIENTID used a machine credential, or fell back to a regular > user's credential if no keytab is available. > > On clients that don't have a keytab, performing SETCLIENTID early > means there's no user credential to fall back on, since no regular > user has kinit'd yet. 05f4c350 seems to have broken the ability > to mount with sec=krb5 on clients that don't have a keytab in > kernels 3.7 - 3.9. > > To address this regression, commit 4edaa308 (NFS: Use "krb5i" to > establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013, > was merged in 3.10. This commit forces the NFS client to fall back > to AUTH_SYS for lease management operations if no keytab is > available. > > Neil Brown noticed that, since root is required to kinit to do a > sec=krb5 mount when a client doesn't have a keytab, we can try to > use root's Kerberos credential before AUTH_SYS. > > Now, when determining a principal and flavor to use for lease > management, the NFS client tries in this order: > > 1. Flavor: AUTH_GSS, krb5i > Principal: service principal (via keytab) > > 2. Flavor: AUTH_GSS, krb5i > Principal: user principal established for UID 0 (via kinit) > > 3. Flavor: AUTH_SYS > Principal: UID 0 / GID 0 > > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > fs/nfs/nfs4proc.c | 21 ++++++++++++++------- > fs/nfs/nfs4state.c | 16 ++++++++-------- > 2 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 5ba38b3..4d76fae 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, > dprintk("NFS call setclientid auth=%s, '%.*s'\n", > clp->cl_rpcclient->cl_auth->au_ops->au_name, > setclientid.sc_name_len, setclientid.sc_name); > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); We are certainly _not_ resurrecting the RPC_TASK_ROOTCREDS abomination. For one thing is overrides the machine creds that are what we really want to use when available. NACK... > dprintk("NFS reply setclientid: %d\n", status); > return status; > } > @@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp, > dprintk("NFS call setclientid_confirm auth=%s, (client ID %llx)\n", > clp->cl_rpcclient->cl_auth->au_ops->au_name, > clp->cl_clientid); > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > dprintk("NFS reply setclientid_confirm: %d\n", status); > return status; > } > @@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred > goto out; > } > > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > if (status == 0) { > if (memcmp(res.session->sess_id.data, > clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) { > @@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred) > goto out_server_scope; > } > > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > if (status == 0) > status = nfs4_check_cl_exchange_flags(res.flags); > > @@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp, > }; > int status; > > - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > if (status) > dprintk("NFS: Got error %d from the server %s on " > "DESTROY_CLIENTID.", status, clp->cl_hostname); > @@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp, > nfs4_init_channel_attrs(&args); > args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN); > > - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > > if (!status) { > /* Verify the session's negotiated channel_attrs values */ > @@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session, > if (session->clp->cl_cons_state != NFS_CS_READY) > return status; > > - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); > + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, > + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > > if (status) > dprintk("NFS: Got error %d from the server on DESTROY_SESSION. " > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c > index 4949ce5..d8068ab 100644 > --- a/fs/nfs/nfs4state.c > +++ b/fs/nfs/nfs4state.c > @@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp) > int status; > > cred = ops->get_clid_cred(clp); > - if (cred == NULL) > - return -ENOENT; > status = ops->establish_clid(clp, cred); > - put_rpccred(cred); > + if (cred) > + put_rpccred(cred); > if (status != 0) > return status; > pnfs_destroy_all_layouts(clp); > @@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp, > again: > status = -ENOENT; > cred = ops->get_clid_cred(clp); > - if (cred == NULL) > - goto out_unlock; > - > status = ops->detect_trunking(clp, result, cred); > - put_rpccred(cred); > + if (cred) > + put_rpccred(cred); > switch (status) { > case 0: > break; > @@ -1902,6 +1899,10 @@ again: > __func__, status); > goto again; > case -EACCES: > + if (clp->cl_machine_cred != NULL) { > + nfs4_clear_machine_cred(clp); > + goto again; > + } > if (i++) > break; > case -NFS4ERR_CLID_INUSE: > @@ -1935,7 +1936,6 @@ again: > status = -EIO; > } > > -out_unlock: > mutex_unlock(&nfs_clid_init_mutex); > dprintk("NFS: %s: status = %d\n", __func__, status); > return status; > > -- > 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 -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@netapp.com www.netapp.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing 2013-06-07 19:52 ` Myklebust, Trond @ 2013-06-07 20:04 ` Chuck Lever 0 siblings, 0 replies; 6+ messages in thread From: Chuck Lever @ 2013-06-07 20:04 UTC (permalink / raw) To: Myklebust, Trond; +Cc: linux-nfs@vger.kernel.org On Jun 7, 2013, at 3:52 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote: > On Fri, 2013-06-07 at 15:45 -0400, Chuck Lever wrote: >> Commit 05f4c350 "NFS: Discover NFSv4 server trunking when mounting" >> Fri Sep 14 17:24:32 2012 introduced Uniform Client String support, >> which forces our NFS client to establish a client ID immediately >> during a mount operation rather than waiting until a user wants to >> open a file. >> >> Normally machine credentials (eg. from a keytab) are used to perform >> a mount operation that is protected by Kerberos. Before 05fc350, >> SETCLIENTID used a machine credential, or fell back to a regular >> user's credential if no keytab is available. >> >> On clients that don't have a keytab, performing SETCLIENTID early >> means there's no user credential to fall back on, since no regular >> user has kinit'd yet. 05f4c350 seems to have broken the ability >> to mount with sec=krb5 on clients that don't have a keytab in >> kernels 3.7 - 3.9. >> >> To address this regression, commit 4edaa308 (NFS: Use "krb5i" to >> establish NFSv4 state whenever possible), Sat Mar 16 15:56:20 2013, >> was merged in 3.10. This commit forces the NFS client to fall back >> to AUTH_SYS for lease management operations if no keytab is >> available. >> >> Neil Brown noticed that, since root is required to kinit to do a >> sec=krb5 mount when a client doesn't have a keytab, we can try to >> use root's Kerberos credential before AUTH_SYS. >> >> Now, when determining a principal and flavor to use for lease >> management, the NFS client tries in this order: >> >> 1. Flavor: AUTH_GSS, krb5i >> Principal: service principal (via keytab) >> >> 2. Flavor: AUTH_GSS, krb5i >> Principal: user principal established for UID 0 (via kinit) >> >> 3. Flavor: AUTH_SYS >> Principal: UID 0 / GID 0 >> >> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> >> --- >> fs/nfs/nfs4proc.c | 21 ++++++++++++++------- >> fs/nfs/nfs4state.c | 16 ++++++++-------- >> 2 files changed, 22 insertions(+), 15 deletions(-) >> >> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c >> index 5ba38b3..4d76fae 100644 >> --- a/fs/nfs/nfs4proc.c >> +++ b/fs/nfs/nfs4proc.c >> @@ -4356,7 +4356,8 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, >> dprintk("NFS call setclientid auth=%s, '%.*s'\n", >> clp->cl_rpcclient->cl_auth->au_ops->au_name, >> setclientid.sc_name_len, setclientid.sc_name); >> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); > > We are certainly _not_ resurrecting the RPC_TASK_ROOTCREDS abomination. Maybe it should be removed then? I don't see any other consumers (even NFS SWAP). > For one thing is overrides the machine creds that are what we really > want to use when available. I don't see how it would override machine creds...? rpcauth_bind_root_cred is called _only_ if the passed-in cred is NULL. The first SETCLIENTID try hands in an rpc_cred, so the root cred is not used. If the first try fails, nfs4_clear_machine_cred() clears that rpc_cred, and SETCLIENTID tries with a NULL cred, which should try to bind to root's cred. If I can figure out another way to obtain a root cred, is this general approach acceptable? Do you have a recommended approach for obtaining the UID 0 cred? > >> dprintk("NFS reply setclientid: %d\n", status); >> return status; >> } >> @@ -4383,7 +4384,8 @@ int nfs4_proc_setclientid_confirm(struct nfs_client *clp, >> dprintk("NFS call setclientid_confirm auth=%s, (client ID %llx)\n", >> clp->cl_rpcclient->cl_auth->au_ops->au_name, >> clp->cl_clientid); >> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> dprintk("NFS reply setclientid_confirm: %d\n", status); >> return status; >> } >> @@ -5461,7 +5463,8 @@ int nfs4_proc_bind_conn_to_session(struct nfs_client *clp, struct rpc_cred *cred >> goto out; >> } >> >> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> if (status == 0) { >> if (memcmp(res.session->sess_id.data, >> clp->cl_session->sess_id.data, NFS4_MAX_SESSIONID_LEN)) { >> @@ -5545,7 +5548,8 @@ int nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred) >> goto out_server_scope; >> } >> >> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> if (status == 0) >> status = nfs4_check_cl_exchange_flags(res.flags); >> >> @@ -5605,7 +5609,8 @@ static int _nfs4_proc_destroy_clientid(struct nfs_client *clp, >> }; >> int status; >> >> - status = rpc_call_sync(clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> if (status) >> dprintk("NFS: Got error %d from the server %s on " >> "DESTROY_CLIENTID.", status, clp->cl_hostname); >> @@ -5871,7 +5876,8 @@ static int _nfs4_proc_create_session(struct nfs_client *clp, >> nfs4_init_channel_attrs(&args); >> args.flags = (SESSION4_PERSIST | SESSION4_BACK_CHAN); >> >> - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> >> if (!status) { >> /* Verify the session's negotiated channel_attrs values */ >> @@ -5934,7 +5940,8 @@ int nfs4_proc_destroy_session(struct nfs4_session *session, >> if (session->clp->cl_cons_state != NFS_CS_READY) >> return status; >> >> - status = rpc_call_sync(session->clp->cl_rpcclient, &msg, RPC_TASK_TIMEOUT); >> + status = rpc_call_sync(session->clp->cl_rpcclient, &msg, >> + RPC_TASK_TIMEOUT|RPC_TASK_ROOTCREDS); >> >> if (status) >> dprintk("NFS: Got error %d from the server on DESTROY_SESSION. " >> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c >> index 4949ce5..d8068ab 100644 >> --- a/fs/nfs/nfs4state.c >> +++ b/fs/nfs/nfs4state.c >> @@ -1811,10 +1811,9 @@ static int nfs4_establish_lease(struct nfs_client *clp) >> int status; >> >> cred = ops->get_clid_cred(clp); >> - if (cred == NULL) >> - return -ENOENT; >> status = ops->establish_clid(clp, cred); >> - put_rpccred(cred); >> + if (cred) >> + put_rpccred(cred); >> if (status != 0) >> return status; >> pnfs_destroy_all_layouts(clp); >> @@ -1885,11 +1884,9 @@ int nfs4_discover_server_trunking(struct nfs_client *clp, >> again: >> status = -ENOENT; >> cred = ops->get_clid_cred(clp); >> - if (cred == NULL) >> - goto out_unlock; >> - >> status = ops->detect_trunking(clp, result, cred); >> - put_rpccred(cred); >> + if (cred) >> + put_rpccred(cred); >> switch (status) { >> case 0: >> break; >> @@ -1902,6 +1899,10 @@ again: >> __func__, status); >> goto again; >> case -EACCES: >> + if (clp->cl_machine_cred != NULL) { >> + nfs4_clear_machine_cred(clp); >> + goto again; >> + } >> if (i++) >> break; >> case -NFS4ERR_CLID_INUSE: >> @@ -1935,7 +1936,6 @@ again: >> status = -EIO; >> } >> >> -out_unlock: >> mutex_unlock(&nfs_clid_init_mutex); >> dprintk("NFS: %s: status = %d\n", __func__, status); >> return status; >> >> -- >> 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 > > > -- > Trond Myklebust > Linux NFS client maintainer > > NetApp > Trond.Myklebust@netapp.com > www.netapp.com -- Chuck Lever chuck[dot]lever[at]oracle[dot]com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-07 20:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-07 19:44 [PATCH 0/3] Use root user credential for lease management Chuck Lever 2013-06-07 19:44 ` [PATCH 1/3] NFS: Never use user credentials for lease renewal Chuck Lever 2013-06-07 19:45 ` [PATCH 2/3] NFS: Partially revert commit 845cbceb Chuck Lever 2013-06-07 19:45 ` [PATCH 3/3] NFS: Use root's credential for lease management when keytab is missing Chuck Lever 2013-06-07 19:52 ` Myklebust, Trond 2013-06-07 20:04 ` Chuck Lever
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).