On Aug 7, 2013, at 8:58 PM, "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx>
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@xxxxxxxxxx> wrote:
>> On Wed, 2013-08-07 at 16:09 -0400, andros@xxxxxxxxxx wrote:
>>> From: Andy Adamson <andros@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
