| Message ID | 1375923513.26965.2.camel@leira.trondhjem.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Aug 7, 2013, at 8:58 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote: > On Wed, 2013-08-07 at 17:26 -0400, Andy Adamson wrote: >> >> >> >> On Wed, Aug 7, 2013 at 4:32 PM, Myklebust, Trond >> <Trond.Myklebust@netapp.com> wrote: >> 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)? >> >> >> No. It shuts down a _clone_ of the client that still belongs to >> NFS_SERVER(dir). >> > > OK. However how about the case where rpc_clone_client() returns an > error? As far as I can tell neither the fix nor the original code deals > with that case. > How about something like the following, where we defer the clone until > we know that it might be needed? Looks good. Testing verifies it works. -->Andy > > -- > Trond Myklebust > Linux NFS client maintainer > > NetApp > Trond.Myklebust@netapp.com > www.netapp.com > <0001-NFSv4-Fix-up-nfs4_proc_lookup_mountpoint.patch> -- 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
From b72888cb0ba63b2dfc6c8d3cd78a7fea584bebc6 Mon Sep 17 00:00:00 2001 From: Trond Myklebust <Trond.Myklebust@netapp.com> Date: Wed, 7 Aug 2013 20:38:07 -0400 Subject: [PATCH] NFSv4: Fix up nfs4_proc_lookup_mountpoint Currently, we do not check the return value of client = rpc_clone_client(), nor do we shut down the resulting cloned rpc_clnt in the case where a NFS4ERR_WRONGSEC has caused nfs4_proc_lookup_common() to replace the original value of 'client' (causing a memory leak). Fix both issues and simplify the code by moving the call to rpc_clone_client() until after nfs4_proc_lookup_common() has done its business. Reported-by: Andy Adamson <andros@netapp.com> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com> --- fs/nfs/nfs4proc.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index cf11799..108a774 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3071,15 +3071,13 @@ struct rpc_clnt * nfs4_proc_lookup_mountpoint(struct inode *dir, struct qstr *name, struct nfs_fh *fhandle, struct nfs_fattr *fattr) { + struct rpc_clnt *client = NFS_CLIENT(dir); int status; - struct rpc_clnt *client = rpc_clone_client(NFS_CLIENT(dir)); status = nfs4_proc_lookup_common(&client, dir, name, fhandle, fattr, NULL); - if (status < 0) { - rpc_shutdown_client(client); + if (status < 0) return ERR_PTR(status); - } - return client; + return (client == NFS_CLIENT(dir)) ? rpc_clone_client(client) : client; } static int _nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry) -- 1.8.3.1