diff mbox

[Version,2,1/6] NFSv4.1 Fix an rpc client leak

Message ID 1375906192-1988-1-git-send-email-andros@netapp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andy Adamson Aug. 7, 2013, 8:09 p.m. UTC 
From: Andy Adamson <andros@netapp.com>

nfs4_proc_lookup_mountpoint clones an rpc client to perform a lookup across
a mountpoint. If the security of the mountpoint is different than that of
the clonded rpc client, a secinfo query is sent, and if successful, a new
rpc client is cloned an returned to nfs4_proc_lookup_mountpoint replacing
the original cloned client pointer. In this case, the originoal cloned client
is not reaped - which results in a leak of multiple rpc clients, as the
parent of the original cloned client will not be dereferenced, and it's
parent will not be dereferenced...

Signed-off-by: Andy Adamson <andros@netapp.com>
---
 fs/nfs/nfs4proc.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Trond Myklebust Aug. 7, 2013, 8:32 p.m. UTC | #1 
On Wed, 2013-08-07 at 16:09 -0400, andros@netapp.com wrote:
> From: Andy Adamson <andros@netapp.com>

> 

> nfs4_proc_lookup_mountpoint clones an rpc client to perform a lookup across

> a mountpoint. If the security of the mountpoint is different than that of

> the clonded rpc client, a secinfo query is sent, and if successful, a new

> rpc client is cloned an returned to nfs4_proc_lookup_mountpoint replacing

> the original cloned client pointer. In this case, the originoal cloned client

> is not reaped - which results in a leak of multiple rpc clients, as the

> parent of the original cloned client will not be dereferenced, and it's

> parent will not be dereferenced...

> 

> Signed-off-by: Andy Adamson <andros@netapp.com>

> ---

>  fs/nfs/nfs4proc.c | 3 +++

>  1 file changed, 3 insertions(+)

> 

> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c

> index 0e64ccc..ee30a4f 100644

> --- a/fs/nfs/nfs4proc.c

> +++ b/fs/nfs/nfs4proc.c

> @@ -3073,12 +3073,15 @@ nfs4_proc_lookup_mountpoint(struct inode *dir, struct qstr *name,

>  {

>  	int status;

>  	struct rpc_clnt *client = rpc_clone_client(NFS_CLIENT(dir));

> +	struct rpc_clnt *clnt = client;

>  

>  	status = nfs4_proc_lookup_common(&client, dir, name, fhandle, fattr, NULL);

>  	if (status < 0) {

>  		rpc_shutdown_client(client);

>  		return ERR_PTR(status);

>  	}

> +	if (clnt != client)

> +		rpc_shutdown_client(clnt);

>  	return client;

>  }

>  

Wouldn't that shut down the client that still belongs to
NFS_SERVER(dir)?


-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
diff mbox

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0e64ccc..ee30a4f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3073,12 +3073,15 @@  nfs4_proc_lookup_mountpoint(struct inode *dir, struct qstr *name,
 {
 	int status;
 	struct rpc_clnt *client = rpc_clone_client(NFS_CLIENT(dir));
+	struct rpc_clnt *clnt = client;
 
 	status = nfs4_proc_lookup_common(&client, dir, name, fhandle, fattr, NULL);
 	if (status < 0) {
 		rpc_shutdown_client(client);
 		return ERR_PTR(status);
 	}
+	if (clnt != client)
+		rpc_shutdown_client(clnt);
 	return client;
 }