Version 2 takes a new approach that Trond requested: we rely on the
cl_machine_cred to have a reference held for the life of the nfs_client
structure, so the sp4 stuff doesn't need to do any get/put_rpccred.
There's also two new patches:
- "fix SECINFO* use of put_rpccred" - recent changes (by me!) called
put_rpccred of rpc_message.rpc_cred, but this value can change when
nfs4_state_protect() is called. I searched through the rest of the client
source and couldn't find another place where this happens.
- WARN_ON -> WARN_ON_ONCE - oops.
Weston Andros Adamson (4):
NFSv4.1: sp4_mach_cred: ask for WRITE and COMMIT
NFSv4.1: fix SECINFO* use of put_rpccred
NFSv4.1: sp4_mach_cred: no need to ref count creds
NFSv4.1: sp4_mach_cred: WARN_ON -> WARN_ON_ONCE
fs/nfs/nfs4_fs.h | 10 +++++-----
fs/nfs/nfs4proc.c | 22 ++++++++++++++--------
2 files changed, 19 insertions(+), 13 deletions(-)
--
1.7.12.4 (Apple Git-37)
Request SP4_MACH_CRED WRITE and COMMIT support in spo_must_allow list --
they're already supported by the client.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 39b6cf2..dc2d27b 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6151,11 +6151,13 @@ static const struct nfs41_state_protection nfs4_sp4_mach_cred_request = {
},
.allow.u.words = {
[0] = 1 << (OP_CLOSE) |
- 1 << (OP_LOCKU),
+ 1 << (OP_LOCKU) |
+ 1 << (OP_COMMIT),
[1] = 1 << (OP_SECINFO - 32) |
1 << (OP_SECINFO_NO_NAME - 32) |
1 << (OP_TEST_STATEID - 32) |
- 1 << (OP_FREE_STATEID - 32)
+ 1 << (OP_FREE_STATEID - 32) |
+ 1 << (OP_WRITE - 32)
}
};
--
1.7.12.4 (Apple Git-37)
Recent SP4_MACH_CRED changes allows rpc_message.rpc_cred to change,
so keep a separate pointer to the machine cred for put_rpccred.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index dc2d27b..989bb9d 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -6001,10 +6001,12 @@ static int _nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, struct
.rpc_resp = &res,
};
struct rpc_clnt *clnt = NFS_SERVER(dir)->client;
+ struct rpc_cred *cred = NULL;
if (use_integrity) {
clnt = NFS_SERVER(dir)->nfs_client->cl_rpcclient;
- msg.rpc_cred = nfs4_get_clid_cred(NFS_SERVER(dir)->nfs_client);
+ cred = nfs4_get_clid_cred(NFS_SERVER(dir)->nfs_client);
+ msg.rpc_cred = cred;
}
dprintk("NFS call secinfo %s\n", name->name);
@@ -6016,8 +6018,8 @@ static int _nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, struct
&res.seq_res, 0);
dprintk("NFS reply secinfo: %d\n", status);
- if (msg.rpc_cred)
- put_rpccred(msg.rpc_cred);
+ if (cred)
+ put_rpccred(cred);
return status;
}
@@ -7498,11 +7500,13 @@ _nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle,
.rpc_resp = &res,
};
struct rpc_clnt *clnt = server->client;
+ struct rpc_cred *cred = NULL;
int status;
if (use_integrity) {
clnt = server->nfs_client->cl_rpcclient;
- msg.rpc_cred = nfs4_get_clid_cred(server->nfs_client);
+ cred = nfs4_get_clid_cred(server->nfs_client);
+ msg.rpc_cred = cred;
}
dprintk("--> %s\n", __func__);
@@ -7510,8 +7514,8 @@ _nfs41_proc_secinfo_no_name(struct nfs_server *server, struct nfs_fh *fhandle,
&res.seq_res, 0);
dprintk("<-- %s status=%d\n", __func__, status);
- if (msg.rpc_cred)
- put_rpccred(msg.rpc_cred);
+ if (cred)
+ put_rpccred(cred);
return status;
}
--
1.7.12.4 (Apple Git-37)
Do we need to do a get_rpccred() after looking up the machine cred in
nfs_alloc_client()? Where does our guaranteed reference come from?
On Tue, Sep 10, 2013 at 6:44 PM, Weston Andros Adamson <[email protected]> wrote:
> The cl_machine_cred doesn't need to be reference counted here -
> a reference is held is for the lifetime of the struct nfs_client.
> Also, no need to put_rpccred the rpc_message.rpc_cred.
>
> Signed-off-by: Weston Andros Adamson <[email protected]>
> ---
> fs/nfs/nfs4_fs.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index f520a11..07a8aa9 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -279,10 +279,10 @@ _nfs4_state_protect(struct nfs_client *clp, unsigned long sp4_mode,
> if (test_bit(sp4_mode, &clp->cl_sp4_flags)) {
> spin_lock(&clp->cl_lock);
> if (clp->cl_machine_cred != NULL)
> - newcred = get_rpccred(clp->cl_machine_cred);
> + /* don't call get_rpccred on the machine cred -
> + * a reference will be held for life of clp */
> + newcred = clp->cl_machine_cred;
> spin_unlock(&clp->cl_lock);
> - if (msg->rpc_cred)
> - put_rpccred(msg->rpc_cred);
> msg->rpc_cred = newcred;
>
> flavor = clp->cl_rpcclient->cl_auth->au_flavor;
> --
> 1.7.12.4 (Apple Git-37)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
No need to spam the logs
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4_fs.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 07a8aa9..28842ab 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -286,8 +286,8 @@ _nfs4_state_protect(struct nfs_client *clp, unsigned long sp4_mode,
msg->rpc_cred = newcred;
flavor = clp->cl_rpcclient->cl_auth->au_flavor;
- WARN_ON(flavor != RPC_AUTH_GSS_KRB5I &&
- flavor != RPC_AUTH_GSS_KRB5P);
+ WARN_ON_ONCE(flavor != RPC_AUTH_GSS_KRB5I &&
+ flavor != RPC_AUTH_GSS_KRB5P);
*clntp = clp->cl_rpcclient;
return true;
--
1.7.12.4 (Apple Git-37)
The cl_machine_cred doesn't need to be reference counted here -
a reference is held is for the lifetime of the struct nfs_client.
Also, no need to put_rpccred the rpc_message.rpc_cred.
Signed-off-by: Weston Andros Adamson <[email protected]>
---
fs/nfs/nfs4_fs.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index f520a11..07a8aa9 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -279,10 +279,10 @@ _nfs4_state_protect(struct nfs_client *clp, unsigned long sp4_mode,
if (test_bit(sp4_mode, &clp->cl_sp4_flags)) {
spin_lock(&clp->cl_lock);
if (clp->cl_machine_cred != NULL)
- newcred = get_rpccred(clp->cl_machine_cred);
+ /* don't call get_rpccred on the machine cred -
+ * a reference will be held for life of clp */
+ newcred = clp->cl_machine_cred;
spin_unlock(&clp->cl_lock);
- if (msg->rpc_cred)
- put_rpccred(msg->rpc_cred);
msg->rpc_cred = newcred;
flavor = clp->cl_rpcclient->cl_auth->au_flavor;
--
1.7.12.4 (Apple Git-37)