2014-06-12 19:02:49

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 0/1] NFS: Fix SECINFO processing regression

From: Andy Adamson <[email protected]>

Sorry for the repeat send - there is only one patch in Version 2...


Iterate over multiple flavors returned by SECINFO. If RPC_AUTH_GSS is in the
secinfo list, fully test (get and auth, credential, and gss_context)

Fix some error paths.

Version 2:
- Responded to Trond's comments. Cloned an RPC client for each flavor to test
which creates an rpc_auth. Use rpcauth_lookupcred() to create a cred for the
user (and gss_context for gss).

- Removed nfs4_negotiate_security from nfs4_submount as is was just called
from nfs4_proc_lookup_mountpoint.


Testing: Minimal testing of the nfs_test_gss_flavor. Will run more tests after
the June 2014 bakeathon

Note: Can supply a patch for SECINFO_NO_name to use nfs4_negotiate_security.

-->Andy


Andy Adamson (1):
NFS NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support

fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4namespace.c | 103 ++++++++++++++++++++++++++++---------------------
fs/nfs/nfs4proc.c | 2 +-
net/sunrpc/auth.c | 1 +
4 files changed, 61 insertions(+), 47 deletions(-)

--
1.8.3.1



2014-06-12 19:02:39

by Andy Adamson

[permalink] [raw]
Subject: [PATCH Version 2 1/1] NFS NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support

From: Andy Adamson <[email protected]>

Fix nfs4_negotiate_security to create an rpc_clnt used to test each SECINFO
returned pseudoflavor. Check credential creation (and gss_context creation)
which is important for RPC_AUTH_GSS pseudoflavors which can fail for multiple
reasons including mis-configuration.

Don't call nfs4_negotiate in nfs4_submount as it was just called by
nfs4_proc_lookup_mountpoint (nfs4_proc_lookup_common)

Signed-off-by: Andy Adamson <[email protected]>

Conflicts:
fs/nfs/nfs4namespace.c
---
fs/nfs/nfs4_fs.h | 2 +-
fs/nfs/nfs4namespace.c | 103 ++++++++++++++++++++++++++++---------------------
fs/nfs/nfs4proc.c | 2 +-
net/sunrpc/auth.c | 1 +
4 files changed, 61 insertions(+), 47 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index f63cb87..ba2affa 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -230,7 +230,7 @@ int nfs_atomic_open(struct inode *, struct dentry *, struct file *,
extern struct file_system_type nfs4_fs_type;

/* nfs4namespace.c */
-struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *, struct inode *, struct qstr *);
+struct rpc_clnt *nfs4_negotiate_security(struct rpc_clnt *, struct inode *, struct qstr *);
struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
struct nfs_fh *, struct nfs_fattr *);
int nfs4_replace_transport(struct nfs_server *server,
diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
index 3d5dbf8..5517f02 100644
--- a/fs/nfs/nfs4namespace.c
+++ b/fs/nfs/nfs4namespace.c
@@ -139,19 +139,28 @@ static size_t nfs_parse_server_name(char *string, size_t len,
* @server: NFS server struct
* @flavors: List of security tuples returned by SECINFO procedure
*
- * Return the pseudoflavor of the first security mechanism in
- * "flavors" that is locally supported. Return RPC_AUTH_UNIX if
- * no matching flavor is found in the array. The "flavors" array
+ * Return an rpc client that uses the first security mechanism in
+ * "flavors" that is locally supported. The "flavors" array
* is searched in the order returned from the server, per RFC 3530
- * recommendation.
+ * recommendation and each flavor is checked for membership in the
+ * sec= mount option list if it exists.
+ *
+ * Return -EPERM if no matching flavor is found in the array.
+ *
+ * Please call rpc_shutdown_client() when you are done with this rpc client.
+ *
*/
-static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
+static struct rpc_clnt *nfs_find_best_sec(struct rpc_clnt *clnt,
+ struct nfs_server *server,
struct nfs4_secinfo_flavors *flavors)
{
- rpc_authflavor_t pseudoflavor;
+ rpc_authflavor_t pflavor;
struct nfs4_secinfo4 *secinfo;
+ struct rpc_clnt *new;
unsigned int i;
+ struct rpc_cred *cred;

+ new = ERR_PTR(-EPERM);
for (i = 0; i < flavors->num_flavors; i++) {
secinfo = &flavors->flavors[i];

@@ -159,62 +168,71 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
case RPC_AUTH_NULL:
case RPC_AUTH_UNIX:
case RPC_AUTH_GSS:
- pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
+ pflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
&secinfo->flavor_info);
- /* make sure pseudoflavor matches sec= mount opt */
- if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
- nfs_auth_info_match(&server->auth_info,
- pseudoflavor))
- return pseudoflavor;
- break;
+ /* does the pseudoflavor match a sec= mount opt? */
+ if (pflavor != RPC_AUTH_MAXFLAVOR &&
+ nfs_auth_info_match(&server->auth_info, pflavor)) {
+
+ /* Cloning creates an rpc_auth for the flavor */
+ new = rpc_clone_client_set_auth(clnt, pflavor);
+ if (IS_ERR(new))
+ continue;
+ /**
+ * Check that the user actually can use the
+ * flavor. This is mostly for RPC_AUTH_GSS
+ * where cr_init obtains a gss context
+ */
+ cred = rpcauth_lookupcred(new->cl_auth, 0);
+ if (IS_ERR(cred)) {
+ rpc_shutdown_client(new);
+ continue;
+ }
+ put_rpccred(cred);
+ break;
+ }
}
}
-
- /* if there were any sec= options then nothing matched */
- if (server->auth_info.flavor_len > 0)
- return -EPERM;
-
- return RPC_AUTH_UNIX;
+ return new;
}

-static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name)
+/**
+ * nfs4_negotiate_security - in response to an NFS4ERR_WRONGSEC on lookup,
+ * return an rpc_clnt that uses the best available security flavor with
+ * respect to the secinfo flavor list and the sec= mount options.
+ *
+ * @clnt: RPC client to clone
+ * @inode: directory inode
+ * @name: lookup name
+ *
+ * Please call rpc_shutdown_client() when you are done with this rpc client.
+ */
+struct rpc_clnt *
+nfs4_negotiate_security(struct rpc_clnt *clnt, struct inode *inode,
+ struct qstr *name)
{
struct page *page;
struct nfs4_secinfo_flavors *flavors;
- rpc_authflavor_t flavor;
+ struct rpc_clnt *new = ERR_PTR(-ENOMEM);
int err;

page = alloc_page(GFP_KERNEL);
if (!page)
- return -ENOMEM;
+ return new;
+
flavors = page_address(page);

err = nfs4_proc_secinfo(inode, name, flavors);
if (err < 0) {
- flavor = err;
+ new = ERR_PTR(err);;
goto out;
}

- flavor = nfs_find_best_sec(NFS_SERVER(inode), flavors);
+ new = nfs_find_best_sec(clnt, NFS_SERVER(inode), flavors);

out:
put_page(page);
- return flavor;
-}
-
-/*
- * Please call rpc_shutdown_client() when you are done with this client.
- */
-struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *clnt, struct inode *inode,
- struct qstr *name)
-{
- rpc_authflavor_t flavor;
-
- flavor = nfs4_negotiate_security(inode, name);
- if ((int)flavor < 0)
- return ERR_PTR((int)flavor);
-
- return rpc_clone_client_set_auth(clnt, flavor);
+ return new;
}

static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
@@ -397,11 +415,6 @@ struct vfsmount *nfs4_submount(struct nfs_server *server, struct dentry *dentry,

if (client->cl_auth->au_flavor != flavor)
flavor = client->cl_auth->au_flavor;
- else {
- rpc_authflavor_t new = nfs4_negotiate_security(dir, name);
- if ((int)new >= 0)
- flavor = new;
- }
mnt = nfs_do_submount(dentry, fh, fattr, flavor);
out:
rpc_shutdown_client(client);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 68dd81e..fe14063 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3247,7 +3247,7 @@ static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir,
err = -EPERM;
if (client != *clnt)
goto out;
- client = nfs4_create_sec_client(client, dir, name);
+ client = nfs4_negotiate_security(client, dir, name);
if (IS_ERR(client))
return PTR_ERR(client);

diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
index 5285ead..1d97e19 100644
--- a/net/sunrpc/auth.c
+++ b/net/sunrpc/auth.c
@@ -592,6 +592,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags)
put_group_info(acred.group_info);
return ret;
}
+EXPORT_SYMBOL_GPL(rpcauth_lookupcred);

void
rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
--
1.8.3.1


2014-06-16 17:56:38

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFS NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support

On Thu, 2014-06-12 at 15:02 -0400, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Fix nfs4_negotiate_security to create an rpc_clnt used to test each SECINFO
> returned pseudoflavor. Check credential creation (and gss_context creation)
> which is important for RPC_AUTH_GSS pseudoflavors which can fail for multiple
> reasons including mis-configuration.
>
> Don't call nfs4_negotiate in nfs4_submount as it was just called by
> nfs4_proc_lookup_mountpoint (nfs4_proc_lookup_common)
>
> Signed-off-by: Andy Adamson <[email protected]>
>
> Conflicts:
> fs/nfs/nfs4namespace.c

Queued for 3.17, but I had to fix a couple of issues with
nfs_find_best_sec() first. See below:


> ---
> fs/nfs/nfs4_fs.h | 2 +-
> fs/nfs/nfs4namespace.c | 103 ++++++++++++++++++++++++++++---------------------
> fs/nfs/nfs4proc.c | 2 +-
> net/sunrpc/auth.c | 1 +
> 4 files changed, 61 insertions(+), 47 deletions(-)
>
> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> index f63cb87..ba2affa 100644
> --- a/fs/nfs/nfs4_fs.h
> +++ b/fs/nfs/nfs4_fs.h
> @@ -230,7 +230,7 @@ int nfs_atomic_open(struct inode *, struct dentry *, struct file *,
> extern struct file_system_type nfs4_fs_type;
>
> /* nfs4namespace.c */
> -struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *, struct inode *, struct qstr *);
> +struct rpc_clnt *nfs4_negotiate_security(struct rpc_clnt *, struct inode *, struct qstr *);
> struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
> struct nfs_fh *, struct nfs_fattr *);
> int nfs4_replace_transport(struct nfs_server *server,
> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
> index 3d5dbf8..5517f02 100644
> --- a/fs/nfs/nfs4namespace.c
> +++ b/fs/nfs/nfs4namespace.c
> @@ -139,19 +139,28 @@ static size_t nfs_parse_server_name(char *string, size_t len,
> * @server: NFS server struct
> * @flavors: List of security tuples returned by SECINFO procedure
> *
> - * Return the pseudoflavor of the first security mechanism in
> - * "flavors" that is locally supported. Return RPC_AUTH_UNIX if
> - * no matching flavor is found in the array. The "flavors" array
> + * Return an rpc client that uses the first security mechanism in
> + * "flavors" that is locally supported. The "flavors" array
> * is searched in the order returned from the server, per RFC 3530
> - * recommendation.
> + * recommendation and each flavor is checked for membership in the
> + * sec= mount option list if it exists.
> + *
> + * Return -EPERM if no matching flavor is found in the array.
> + *
> + * Please call rpc_shutdown_client() when you are done with this rpc client.
> + *
> */
> -static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
> +static struct rpc_clnt *nfs_find_best_sec(struct rpc_clnt *clnt,
> + struct nfs_server *server,
> struct nfs4_secinfo_flavors *flavors)
> {
> - rpc_authflavor_t pseudoflavor;
> + rpc_authflavor_t pflavor;
> struct nfs4_secinfo4 *secinfo;
> + struct rpc_clnt *new;
> unsigned int i;
> + struct rpc_cred *cred;
>
> + new = ERR_PTR(-EPERM);
> for (i = 0; i < flavors->num_flavors; i++) {
> secinfo = &flavors->flavors[i];
>
> @@ -159,62 +168,71 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
> case RPC_AUTH_NULL:
> case RPC_AUTH_UNIX:
> case RPC_AUTH_GSS:
> - pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
> + pflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
> &secinfo->flavor_info);
> - /* make sure pseudoflavor matches sec= mount opt */
> - if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
> - nfs_auth_info_match(&server->auth_info,
> - pseudoflavor))
> - return pseudoflavor;
> - break;
> + /* does the pseudoflavor match a sec= mount opt? */
> + if (pflavor != RPC_AUTH_MAXFLAVOR &&
> + nfs_auth_info_match(&server->auth_info, pflavor)) {
> +
> + /* Cloning creates an rpc_auth for the flavor */
> + new = rpc_clone_client_set_auth(clnt, pflavor);
> + if (IS_ERR(new))
> + continue;

If IS_ERR(new), we can end up returning an error which is not EPERM.

> + /**
> + * Check that the user actually can use the
> + * flavor. This is mostly for RPC_AUTH_GSS
> + * where cr_init obtains a gss context
> + */
> + cred = rpcauth_lookupcred(new->cl_auth, 0);
> + if (IS_ERR(cred)) {
> + rpc_shutdown_client(new);
> + continue;
> + }
> + put_rpccred(cred);
> + break;

If IS_ERR(cred), we can end up freeing 'new', and then returning the
pointer to the freed client.

> + }
> }
> }
> -
> - /* if there were any sec= options then nothing matched */
> - if (server->auth_info.flavor_len > 0)
> - return -EPERM;
> -
> - return RPC_AUTH_UNIX;
> + return new;
> }
>
> -static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name)
> +/**
> + * nfs4_negotiate_security - in response to an NFS4ERR_WRONGSEC on lookup,
> + * return an rpc_clnt that uses the best available security flavor with
> + * respect to the secinfo flavor list and the sec= mount options.
> + *
> + * @clnt: RPC client to clone
> + * @inode: directory inode
> + * @name: lookup name
> + *
> + * Please call rpc_shutdown_client() when you are done with this rpc client.
> + */
> +struct rpc_clnt *
> +nfs4_negotiate_security(struct rpc_clnt *clnt, struct inode *inode,
> + struct qstr *name)
> {
> struct page *page;
> struct nfs4_secinfo_flavors *flavors;
> - rpc_authflavor_t flavor;
> + struct rpc_clnt *new = ERR_PTR(-ENOMEM);
> int err;
>
> page = alloc_page(GFP_KERNEL);
> if (!page)
> - return -ENOMEM;
> + return new;
> +
> flavors = page_address(page);
>
> err = nfs4_proc_secinfo(inode, name, flavors);
> if (err < 0) {
> - flavor = err;
> + new = ERR_PTR(err);;
> goto out;
> }
>
> - flavor = nfs_find_best_sec(NFS_SERVER(inode), flavors);
> + new = nfs_find_best_sec(clnt, NFS_SERVER(inode), flavors);
>
> out:
> put_page(page);
> - return flavor;
> -}
> -
> -/*
> - * Please call rpc_shutdown_client() when you are done with this client.
> - */
> -struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *clnt, struct inode *inode,
> - struct qstr *name)
> -{
> - rpc_authflavor_t flavor;
> -
> - flavor = nfs4_negotiate_security(inode, name);
> - if ((int)flavor < 0)
> - return ERR_PTR((int)flavor);
> -
> - return rpc_clone_client_set_auth(clnt, flavor);
> + return new;
> }
>
> static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
> @@ -397,11 +415,6 @@ struct vfsmount *nfs4_submount(struct nfs_server *server, struct dentry *dentry,
>
> if (client->cl_auth->au_flavor != flavor)
> flavor = client->cl_auth->au_flavor;
> - else {
> - rpc_authflavor_t new = nfs4_negotiate_security(dir, name);
> - if ((int)new >= 0)
> - flavor = new;
> - }
> mnt = nfs_do_submount(dentry, fh, fattr, flavor);
> out:
> rpc_shutdown_client(client);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 68dd81e..fe14063 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3247,7 +3247,7 @@ static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir,
> err = -EPERM;
> if (client != *clnt)
> goto out;
> - client = nfs4_create_sec_client(client, dir, name);
> + client = nfs4_negotiate_security(client, dir, name);
> if (IS_ERR(client))
> return PTR_ERR(client);
>
> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
> index 5285ead..1d97e19 100644
> --- a/net/sunrpc/auth.c
> +++ b/net/sunrpc/auth.c
> @@ -592,6 +592,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags)
> put_group_info(acred.group_info);
> return ret;
> }
> +EXPORT_SYMBOL_GPL(rpcauth_lookupcred);
>
> void
> rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,

--
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email protected]



2014-06-16 18:25:45

by Adamson, Andy

[permalink] [raw]
Subject: Re: [PATCH Version 2 1/1] NFS NFS test SECINFO RPC_AUTH_GSS pseudoflavors for support


On Jun 16, 2014, at 1:56 PM, Trond Myklebust <[email protected]> wrote:

> On Thu, 2014-06-12 at 15:02 -0400, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Fix nfs4_negotiate_security to create an rpc_clnt used to test each SECINFO
>> returned pseudoflavor. Check credential creation (and gss_context creation)
>> which is important for RPC_AUTH_GSS pseudoflavors which can fail for multiple
>> reasons including mis-configuration.
>>
>> Don't call nfs4_negotiate in nfs4_submount as it was just called by
>> nfs4_proc_lookup_mountpoint (nfs4_proc_lookup_common)
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>>
>> Conflicts:
>> fs/nfs/nfs4namespace.c
>
> Queued for 3.17, but I had to fix a couple of issues with
> nfs_find_best_sec() first. See below:

Thanks for the fixes which look good.

>
>
>> ---
>> fs/nfs/nfs4_fs.h | 2 +-
>> fs/nfs/nfs4namespace.c | 103 ++++++++++++++++++++++++++++---------------------
>> fs/nfs/nfs4proc.c | 2 +-
>> net/sunrpc/auth.c | 1 +
>> 4 files changed, 61 insertions(+), 47 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
>> index f63cb87..ba2affa 100644
>> --- a/fs/nfs/nfs4_fs.h
>> +++ b/fs/nfs/nfs4_fs.h
>> @@ -230,7 +230,7 @@ int nfs_atomic_open(struct inode *, struct dentry *, struct file *,
>> extern struct file_system_type nfs4_fs_type;
>>
>> /* nfs4namespace.c */
>> -struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *, struct inode *, struct qstr *);
>> +struct rpc_clnt *nfs4_negotiate_security(struct rpc_clnt *, struct inode *, struct qstr *);
>> struct vfsmount *nfs4_submount(struct nfs_server *, struct dentry *,
>> struct nfs_fh *, struct nfs_fattr *);
>> int nfs4_replace_transport(struct nfs_server *server,
>> diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c
>> index 3d5dbf8..5517f02 100644
>> --- a/fs/nfs/nfs4namespace.c
>> +++ b/fs/nfs/nfs4namespace.c
>> @@ -139,19 +139,28 @@ static size_t nfs_parse_server_name(char *string, size_t len,
>> * @server: NFS server struct
>> * @flavors: List of security tuples returned by SECINFO procedure
>> *
>> - * Return the pseudoflavor of the first security mechanism in
>> - * "flavors" that is locally supported. Return RPC_AUTH_UNIX if
>> - * no matching flavor is found in the array. The "flavors" array
>> + * Return an rpc client that uses the first security mechanism in
>> + * "flavors" that is locally supported. The "flavors" array
>> * is searched in the order returned from the server, per RFC 3530
>> - * recommendation.
>> + * recommendation and each flavor is checked for membership in the
>> + * sec= mount option list if it exists.
>> + *
>> + * Return -EPERM if no matching flavor is found in the array.
>> + *
>> + * Please call rpc_shutdown_client() when you are done with this rpc client.
>> + *
>> */
>> -static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
>> +static struct rpc_clnt *nfs_find_best_sec(struct rpc_clnt *clnt,
>> + struct nfs_server *server,
>> struct nfs4_secinfo_flavors *flavors)
>> {
>> - rpc_authflavor_t pseudoflavor;
>> + rpc_authflavor_t pflavor;
>> struct nfs4_secinfo4 *secinfo;
>> + struct rpc_clnt *new;
>> unsigned int i;
>> + struct rpc_cred *cred;
>>
>> + new = ERR_PTR(-EPERM);
>> for (i = 0; i < flavors->num_flavors; i++) {
>> secinfo = &flavors->flavors[i];
>>
>> @@ -159,62 +168,71 @@ static rpc_authflavor_t nfs_find_best_sec(struct nfs_server *server,
>> case RPC_AUTH_NULL:
>> case RPC_AUTH_UNIX:
>> case RPC_AUTH_GSS:
>> - pseudoflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
>> + pflavor = rpcauth_get_pseudoflavor(secinfo->flavor,
>> &secinfo->flavor_info);
>> - /* make sure pseudoflavor matches sec= mount opt */
>> - if (pseudoflavor != RPC_AUTH_MAXFLAVOR &&
>> - nfs_auth_info_match(&server->auth_info,
>> - pseudoflavor))
>> - return pseudoflavor;
>> - break;
>> + /* does the pseudoflavor match a sec= mount opt? */
>> + if (pflavor != RPC_AUTH_MAXFLAVOR &&
>> + nfs_auth_info_match(&server->auth_info, pflavor)) {
>> +
>> + /* Cloning creates an rpc_auth for the flavor */
>> + new = rpc_clone_client_set_auth(clnt, pflavor);
>> + if (IS_ERR(new))
>> + continue;
>
> If IS_ERR(new), we can end up returning an error which is not EPERM.
>
>> + /**
>> + * Check that the user actually can use the
>> + * flavor. This is mostly for RPC_AUTH_GSS
>> + * where cr_init obtains a gss context
>> + */
>> + cred = rpcauth_lookupcred(new->cl_auth, 0);
>> + if (IS_ERR(cred)) {
>> + rpc_shutdown_client(new);
>> + continue;
>> + }
>> + put_rpccred(cred);
>> + break;
>
> If IS_ERR(cred), we can end up freeing 'new', and then returning the
> pointer to the freed client.

ugg. Should have caught this one?

Thanks

?>Andy

>
>> + }
>> }
>> }
>> -
>> - /* if there were any sec= options then nothing matched */
>> - if (server->auth_info.flavor_len > 0)
>> - return -EPERM;
>> -
>> - return RPC_AUTH_UNIX;
>> + return new;
>> }
>>
>> -static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name)
>> +/**
>> + * nfs4_negotiate_security - in response to an NFS4ERR_WRONGSEC on lookup,
>> + * return an rpc_clnt that uses the best available security flavor with
>> + * respect to the secinfo flavor list and the sec= mount options.
>> + *
>> + * @clnt: RPC client to clone
>> + * @inode: directory inode
>> + * @name: lookup name
>> + *
>> + * Please call rpc_shutdown_client() when you are done with this rpc client.
>> + */
>> +struct rpc_clnt *
>> +nfs4_negotiate_security(struct rpc_clnt *clnt, struct inode *inode,
>> + struct qstr *name)
>> {
>> struct page *page;
>> struct nfs4_secinfo_flavors *flavors;
>> - rpc_authflavor_t flavor;
>> + struct rpc_clnt *new = ERR_PTR(-ENOMEM);
>> int err;
>>
>> page = alloc_page(GFP_KERNEL);
>> if (!page)
>> - return -ENOMEM;
>> + return new;
>> +
>> flavors = page_address(page);
>>
>> err = nfs4_proc_secinfo(inode, name, flavors);
>> if (err < 0) {
>> - flavor = err;
>> + new = ERR_PTR(err);;
>> goto out;
>> }
>>
>> - flavor = nfs_find_best_sec(NFS_SERVER(inode), flavors);
>> + new = nfs_find_best_sec(clnt, NFS_SERVER(inode), flavors);
>>
>> out:
>> put_page(page);
>> - return flavor;
>> -}
>> -
>> -/*
>> - * Please call rpc_shutdown_client() when you are done with this client.
>> - */
>> -struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *clnt, struct inode *inode,
>> - struct qstr *name)
>> -{
>> - rpc_authflavor_t flavor;
>> -
>> - flavor = nfs4_negotiate_security(inode, name);
>> - if ((int)flavor < 0)
>> - return ERR_PTR((int)flavor);
>> -
>> - return rpc_clone_client_set_auth(clnt, flavor);
>> + return new;
>> }
>>
>> static struct vfsmount *try_location(struct nfs_clone_mount *mountdata,
>> @@ -397,11 +415,6 @@ struct vfsmount *nfs4_submount(struct nfs_server *server, struct dentry *dentry,
>>
>> if (client->cl_auth->au_flavor != flavor)
>> flavor = client->cl_auth->au_flavor;
>> - else {
>> - rpc_authflavor_t new = nfs4_negotiate_security(dir, name);
>> - if ((int)new >= 0)
>> - flavor = new;
>> - }
>> mnt = nfs_do_submount(dentry, fh, fattr, flavor);
>> out:
>> rpc_shutdown_client(client);
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 68dd81e..fe14063 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -3247,7 +3247,7 @@ static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir,
>> err = -EPERM;
>> if (client != *clnt)
>> goto out;
>> - client = nfs4_create_sec_client(client, dir, name);
>> + client = nfs4_negotiate_security(client, dir, name);
>> if (IS_ERR(client))
>> return PTR_ERR(client);
>>
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index 5285ead..1d97e19 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -592,6 +592,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags)
>> put_group_info(acred.group_info);
>> return ret;
>> }
>> +EXPORT_SYMBOL_GPL(rpcauth_lookupcred);
>>
>> void
>> rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
>
> --
> Trond Myklebust
> Linux NFS client maintainer, PrimaryData
> [email protected]