[1/1] NFS prevent double free in async nfs4_exchange_id
diff mbox

Message ID 20170310191541.38007-1-kolga@netapp.com
State New
Headers show

Commit Message

Olga Kornievskaia March 10, 2017, 7:15 p.m. UTC
Since rpc_task is async, the release function should be called which
will free the impl_id, scope, and owner.

Fixes: 8d89bd70bc9 ("NFS setup async exchange_id")
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfs/nfs4proc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Trond Myklebust March 10, 2017, 8:51 p.m. UTC | #1
On Fri, 2017-03-10 at 14:15 -0500, Olga Kornievskaia wrote:
> Since rpc_task is async, the release function should be called which

> will free the impl_id, scope, and owner.

> 

> Fixes: 8d89bd70bc9 ("NFS setup async exchange_id")

> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>

> ---

>  fs/nfs/nfs4proc.c | 6 ++----

>  1 file changed, 2 insertions(+), 4 deletions(-)

> 

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

> index 59be0f7..b77cb6f 100644

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

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

> @@ -7537,10 +7537,8 @@ static int _nfs4_proc_exchange_id(struct

> nfs_client *clp, struct rpc_cred *cred,

>  	task_setup_data.callback_data = calldata;

>  

>  	task = rpc_run_task(&task_setup_data);

> -	if (IS_ERR(task)) {

> -	status = PTR_ERR(task);

> -		goto out_impl_id;

> -	}

> +	if (IS_ERR(task))

> +		return PTR_ERR(task);

>  

>  	if (!xprt) {

>  		status = rpc_wait_for_completion_task(task);


Urgh, yes...

As far as I can see, there is also at least one more use-after-free
issue that was introduced in nfs4_exchange_id_release() by the same
patch. There is also a leak of clp->cl_count in the cases where we
error before calling rpc_run_task().

...and can someone please explain to me what is intended with the line
'status = calldata->rpc_status' in the case where we're not waiting for
completion of the RPC call (i.e. when xprt != NULL)?

Thanks
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

Patch
diff mbox

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 59be0f7..b77cb6f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7537,10 +7537,8 @@  static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	task_setup_data.callback_data = calldata;
 
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task)) {
-	status = PTR_ERR(task);
-		goto out_impl_id;
-	}
+	if (IS_ERR(task))
+		return PTR_ERR(task);
 
 	if (!xprt) {
 		status = rpc_wait_for_completion_task(task);