All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id
@ 2017-03-10 19:15 Olga Kornievskaia
  2017-03-10 20:51 ` Trond Myklebust
  0 siblings, 1 reply; 6+ messages in thread
From: Olga Kornievskaia @ 2017-03-10 19:15 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

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);
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id
  2017-03-10 19:15 [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id Olga Kornievskaia
@ 2017-03-10 20:51 ` Trond Myklebust
  2017-03-10 21:35   ` Olga Kornievskaia
  0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2017-03-10 20:51 UTC (permalink / raw)
  To: anna.schumaker, kolga; +Cc: linux-nfs

T24gRnJpLCAyMDE3LTAzLTEwIGF0IDE0OjE1IC0wNTAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gU2luY2UgcnBjX3Rhc2sgaXMgYXN5bmMsIHRoZSByZWxlYXNlIGZ1bmN0aW9uIHNob3Vs
ZCBiZSBjYWxsZWQgd2hpY2gNCj4gd2lsbCBmcmVlIHRoZSBpbXBsX2lkLCBzY29wZSwgYW5kIG93
bmVyLg0KPiANCj4gRml4ZXM6IDhkODliZDcwYmM5ICgiTkZTIHNldHVwIGFzeW5jIGV4Y2hhbmdl
X2lkIikNCj4gU2lnbmVkLW9mZi1ieTogT2xnYSBLb3JuaWV2c2thaWEgPGtvbGdhQG5ldGFwcC5j
b20+DQo+IC0tLQ0KPiDCoGZzL25mcy9uZnM0cHJvYy5jIHwgNiArKy0tLS0NCj4gwqAxIGZpbGUg
Y2hhbmdlZCwgMiBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygtKQ0KPiANCj4gZGlmZiAtLWdp
dCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gaW5kZXggNTliZTBm
Ny4uYjc3Y2I2ZiAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL25mczRwcm9jLmMNCj4gKysrIGIvZnMv
bmZzL25mczRwcm9jLmMNCj4gQEAgLTc1MzcsMTAgKzc1MzcsOCBAQCBzdGF0aWMgaW50IF9uZnM0
X3Byb2NfZXhjaGFuZ2VfaWQoc3RydWN0DQo+IG5mc19jbGllbnQgKmNscCwgc3RydWN0IHJwY19j
cmVkICpjcmVkLA0KPiDCoAl0YXNrX3NldHVwX2RhdGEuY2FsbGJhY2tfZGF0YSA9IGNhbGxkYXRh
Ow0KPiDCoA0KPiDCoAl0YXNrID0gcnBjX3J1bl90YXNrKCZ0YXNrX3NldHVwX2RhdGEpOw0KPiAt
CWlmIChJU19FUlIodGFzaykpIHsNCj4gLQlzdGF0dXMgPSBQVFJfRVJSKHRhc2spOw0KPiAtCQln
b3RvIG91dF9pbXBsX2lkOw0KPiAtCX0NCj4gKwlpZiAoSVNfRVJSKHRhc2spKQ0KPiArCQlyZXR1
cm4gUFRSX0VSUih0YXNrKTsNCj4gwqANCj4gwqAJaWYgKCF4cHJ0KSB7DQo+IMKgCQlzdGF0dXMg
PSBycGNfd2FpdF9mb3JfY29tcGxldGlvbl90YXNrKHRhc2spOw0KDQpVcmdoLCB5ZXMuLi4NCg0K
QXMgZmFyIGFzIEkgY2FuIHNlZSwgdGhlcmUgaXMgYWxzbyBhdCBsZWFzdCBvbmUgbW9yZSB1c2Ut
YWZ0ZXItZnJlZQ0KaXNzdWUgdGhhdCB3YXMgaW50cm9kdWNlZCBpbiBuZnM0X2V4Y2hhbmdlX2lk
X3JlbGVhc2UoKSBieSB0aGUgc2FtZQ0KcGF0Y2guIFRoZXJlIGlzIGFsc28gYSBsZWFrIG9mIGNs
cC0+Y2xfY291bnQgaW4gdGhlIGNhc2VzIHdoZXJlIHdlDQplcnJvciBiZWZvcmUgY2FsbGluZyBy
cGNfcnVuX3Rhc2soKS4NCg0KLi4uYW5kIGNhbiBzb21lb25lIHBsZWFzZSBleHBsYWluIHRvIG1l
IHdoYXQgaXMgaW50ZW5kZWQgd2l0aCB0aGUgbGluZQ0KJ3N0YXR1cyA9IGNhbGxkYXRhLT5ycGNf
c3RhdHVzJyBpbiB0aGUgY2FzZSB3aGVyZSB3ZSdyZSBub3Qgd2FpdGluZyBmb3INCmNvbXBsZXRp
b24gb2YgdGhlIFJQQyBjYWxsIChpLmUuIHdoZW4geHBydCAhPSBOVUxMKT8NCg0KVGhhbmtzDQog
IFRyb25kDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWlu
ZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id
  2017-03-10 20:51 ` Trond Myklebust
@ 2017-03-10 21:35   ` Olga Kornievskaia
  2017-03-10 21:56     ` Olga Kornievskaia
  0 siblings, 1 reply; 6+ messages in thread
From: Olga Kornievskaia @ 2017-03-10 21:35 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Since rpc_task is async, the release function should be called which
will free the impl_id, scope, and owner.

Trond pointed at 2 more problems:
-- use of client pointer after free in the nfs4_exchangeid_release() function
-- cl_count mismatch if rpc_run_task() isn't run

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 59be0f7..3a79d3a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7426,11 +7426,11 @@ static void nfs4_exchange_id_release(void *data)
 	struct nfs41_exchange_id_data *cdata =
 					(struct nfs41_exchange_id_data *)data;
 
-	nfs_put_client(cdata->args.client);
 	if (cdata->xprt) {
 		xprt_put(cdata->xprt);
 		rpc_clnt_xprt_switch_put(cdata->args.client->cl_rpcclient);
 	}
+	nfs_put_client(cdata->args.client);
 	kfree(cdata->res.impl_id);
 	kfree(cdata->res.server_scope);
 	kfree(cdata->res.server_owner);
@@ -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);
@@ -7558,6 +7556,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 			clp->cl_implid->date.seconds,
 			clp->cl_implid->date.nseconds);
 	dprintk("NFS reply exchange_id: %d\n", status);
+	if (status)
+		nfs_put_client(clp);
 	return status;
 
 out_impl_id:
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id
  2017-03-10 21:35   ` Olga Kornievskaia
@ 2017-03-10 21:56     ` Olga Kornievskaia
  2017-03-11 15:49       ` Trond Myklebust
  0 siblings, 1 reply; 6+ messages in thread
From: Olga Kornievskaia @ 2017-03-10 21:56 UTC (permalink / raw)
  To: Olga Kornievskaia; +Cc: Trond Myklebust, Anna Schumaker, linux-nfs

On Fri, Mar 10, 2017 at 4:35 PM, Olga Kornievskaia <kolga@netapp.com> wrote:
> Since rpc_task is async, the release function should be called which
> will free the impl_id, scope, and owner.
>
> Trond pointed at 2 more problems:
> -- use of client pointer after free in the nfs4_exchangeid_release() function
> -- cl_count mismatch if rpc_run_task() isn't run
>
> Fixes: 8d89bd70bc9 ("NFS setup async exchange_id")
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  fs/nfs/nfs4proc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 59be0f7..3a79d3a 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -7426,11 +7426,11 @@ static void nfs4_exchange_id_release(void *data)
>         struct nfs41_exchange_id_data *cdata =
>                                         (struct nfs41_exchange_id_data *)data;
>
> -       nfs_put_client(cdata->args.client);
>         if (cdata->xprt) {
>                 xprt_put(cdata->xprt);
>                 rpc_clnt_xprt_switch_put(cdata->args.client->cl_rpcclient);
>         }
> +       nfs_put_client(cdata->args.client);
>         kfree(cdata->res.impl_id);
>         kfree(cdata->res.server_scope);
>         kfree(cdata->res.server_owner);
> @@ -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);
> @@ -7558,6 +7556,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
>                         clp->cl_implid->date.seconds,
>                         clp->cl_implid->date.nseconds);
>         dprintk("NFS reply exchange_id: %d\n", status);
> +       if (status)
> +               nfs_put_client(clp);
>         return status;
>
>  out_impl_id:

Urgh. scratch this one, it's causing problems. Will try again.


> --
> 1.8.3.1
>
> --
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id
  2017-03-10 21:56     ` Olga Kornievskaia
@ 2017-03-11 15:49       ` Trond Myklebust
  2017-03-13 14:36         ` [PATCH v2 " Olga Kornievskaia
  0 siblings, 1 reply; 6+ messages in thread
From: Trond Myklebust @ 2017-03-11 15:49 UTC (permalink / raw)
  To: aglo, kolga; +Cc: anna.schumaker, linux-nfs

T24gRnJpLCAyMDE3LTAzLTEwIGF0IDE2OjU2IC0wNTAwLCBPbGdhIEtvcm5pZXZza2FpYSB3cm90
ZToNCj4gT24gRnJpLCBNYXIgMTAsIDIwMTcgYXQgNDozNSBQTSwgT2xnYSBLb3JuaWV2c2thaWEg
PGtvbGdhQG5ldGFwcC5jb20+DQo+IHdyb3RlOg0KPiA+IFNpbmNlIHJwY190YXNrIGlzIGFzeW5j
LCB0aGUgcmVsZWFzZSBmdW5jdGlvbiBzaG91bGQgYmUgY2FsbGVkDQo+ID4gd2hpY2gNCj4gPiB3
aWxsIGZyZWUgdGhlIGltcGxfaWQsIHNjb3BlLCBhbmQgb3duZXIuDQo+ID4gDQo+ID4gVHJvbmQg
cG9pbnRlZCBhdCAyIG1vcmUgcHJvYmxlbXM6DQo+ID4gLS0gdXNlIG9mIGNsaWVudCBwb2ludGVy
IGFmdGVyIGZyZWUgaW4gdGhlDQo+ID4gbmZzNF9leGNoYW5nZWlkX3JlbGVhc2UoKSBmdW5jdGlv
bg0KPiA+IC0tIGNsX2NvdW50IG1pc21hdGNoIGlmIHJwY19ydW5fdGFzaygpIGlzbid0IHJ1bg0K
PiA+IA0KPiA+IEZpeGVzOiA4ZDg5YmQ3MGJjOSAoIk5GUyBzZXR1cCBhc3luYyBleGNoYW5nZV9p
ZCIpDQo+ID4gU2lnbmVkLW9mZi1ieTogT2xnYSBLb3JuaWV2c2thaWEgPGtvbGdhQG5ldGFwcC5j
b20+DQo+ID4gLS0tDQo+ID4gwqBmcy9uZnMvbmZzNHByb2MuYyB8IDEwICsrKysrLS0tLS0NCj4g
PiDCoDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pDQo+ID4g
DQo+ID4gZGlmZiAtLWdpdCBhL2ZzL25mcy9uZnM0cHJvYy5jIGIvZnMvbmZzL25mczRwcm9jLmMN
Cj4gPiBpbmRleCA1OWJlMGY3Li4zYTc5ZDNhIDEwMDY0NA0KPiA+IC0tLSBhL2ZzL25mcy9uZnM0
cHJvYy5jDQo+ID4gKysrIGIvZnMvbmZzL25mczRwcm9jLmMNCj4gPiBAQCAtNzQyNiwxMSArNzQy
NiwxMSBAQCBzdGF0aWMgdm9pZCBuZnM0X2V4Y2hhbmdlX2lkX3JlbGVhc2Uodm9pZA0KPiA+ICpk
YXRhKQ0KPiA+IMKgwqDCoMKgwqDCoMKgwqBzdHJ1Y3QgbmZzNDFfZXhjaGFuZ2VfaWRfZGF0YSAq
Y2RhdGEgPQ0KPiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgKHN0cnVjdA0KPiA+IG5mczQxX2V4
Y2hhbmdlX2lkX2RhdGEgKilkYXRhOw0KPiA+IA0KPiA+IC3CoMKgwqDCoMKgwqDCoG5mc19wdXRf
Y2xpZW50KGNkYXRhLT5hcmdzLmNsaWVudCk7DQo+ID4gwqDCoMKgwqDCoMKgwqDCoGlmIChjZGF0
YS0+eHBydCkgew0KPiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgeHBydF9wdXQo
Y2RhdGEtPnhwcnQpOw0KPiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcnBjX2Ns
bnRfeHBydF9zd2l0Y2hfcHV0KGNkYXRhLT5hcmdzLmNsaWVudC0NCj4gPiA+Y2xfcnBjY2xpZW50
KTsNCj4gPiDCoMKgwqDCoMKgwqDCoMKgfQ0KPiA+ICvCoMKgwqDCoMKgwqDCoG5mc19wdXRfY2xp
ZW50KGNkYXRhLT5hcmdzLmNsaWVudCk7DQo+ID4gwqDCoMKgwqDCoMKgwqDCoGtmcmVlKGNkYXRh
LT5yZXMuaW1wbF9pZCk7DQo+ID4gwqDCoMKgwqDCoMKgwqDCoGtmcmVlKGNkYXRhLT5yZXMuc2Vy
dmVyX3Njb3BlKTsNCj4gPiDCoMKgwqDCoMKgwqDCoMKga2ZyZWUoY2RhdGEtPnJlcy5zZXJ2ZXJf
b3duZXIpOw0KPiA+IEBAIC03NTM3LDEwICs3NTM3LDggQEAgc3RhdGljIGludCBfbmZzNF9wcm9j
X2V4Y2hhbmdlX2lkKHN0cnVjdA0KPiA+IG5mc19jbGllbnQgKmNscCwgc3RydWN0IHJwY19jcmVk
ICpjcmVkLA0KPiA+IMKgwqDCoMKgwqDCoMKgwqB0YXNrX3NldHVwX2RhdGEuY2FsbGJhY2tfZGF0
YSA9IGNhbGxkYXRhOw0KPiA+IA0KPiA+IMKgwqDCoMKgwqDCoMKgwqB0YXNrID0gcnBjX3J1bl90
YXNrKCZ0YXNrX3NldHVwX2RhdGEpOw0KPiA+IC3CoMKgwqDCoMKgwqDCoGlmIChJU19FUlIodGFz
aykpIHsNCj4gPiAtwqDCoMKgwqDCoMKgwqBzdGF0dXMgPSBQVFJfRVJSKHRhc2spOw0KPiA+IC3C
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBnb3RvIG91dF9pbXBsX2lkOw0KPiA+IC3CoMKg
wqDCoMKgwqDCoH0NCj4gPiArwqDCoMKgwqDCoMKgwqBpZiAoSVNfRVJSKHRhc2spKQ0KPiA+ICvC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqByZXR1cm4gUFRSX0VSUih0YXNrKTsNCj4gPiAN
Cj4gPiDCoMKgwqDCoMKgwqDCoMKgaWYgKCF4cHJ0KSB7DQo+ID4gwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqBzdGF0dXMgPSBycGNfd2FpdF9mb3JfY29tcGxldGlvbl90YXNrKHRhc2sp
Ow0KPiA+IEBAIC03NTU4LDYgKzc1NTYsOCBAQCBzdGF0aWMgaW50IF9uZnM0X3Byb2NfZXhjaGFu
Z2VfaWQoc3RydWN0DQo+ID4gbmZzX2NsaWVudCAqY2xwLCBzdHJ1Y3QgcnBjX2NyZWQgKmNyZWQs
DQo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgY2xw
LT5jbF9pbXBsaWQtPmRhdGUuc2Vjb25kcywNCj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBjbHAtPmNsX2ltcGxpZC0+ZGF0ZS5uc2Vjb25kcyk7DQo+
ID4gwqDCoMKgwqDCoMKgwqDCoGRwcmludGsoIk5GUyByZXBseSBleGNoYW5nZV9pZDogJWRcbiIs
IHN0YXR1cyk7DQo+ID4gK8KgwqDCoMKgwqDCoMKgaWYgKHN0YXR1cykNCj4gPiArwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgbmZzX3B1dF9jbGllbnQoY2xwKTsNCg0KVGhpcyBuZWVkcyB0
byBiZSBpbiB0aGUgIm91dF9jYWxsZGF0YSIgZXJyb3IgcGF0aCBvbmx5LiBJdCBpc24ndCBuZWVk
ZWQNCm9uY2Ugd2UndmUgY2FsbGVkIHJwY19ydW5fdGFzaygpLiBPdGhlcndpc2UgdGhlIHBhdGNo
IGxvb2tzIGdvb2QuDQoNCj4gPiDCoMKgwqDCoMKgwqDCoMKgcmV0dXJuIHN0YXR1czsNCj4gPiAN
Cj4gPiDCoG91dF9pbXBsX2lkOg0KPiANCj4gVXJnaC4gc2NyYXRjaCB0aGlzIG9uZSwgaXQncyBj
YXVzaW5nIHByb2JsZW1zLiBXaWxsIHRyeSBhZ2Fpbi4NCj4gDQo+IA0KPiA+IC0tDQo+ID4gMS44
LjMuMQ0KPiA+IA0KPiA+IC0tDQo+ID4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNl
bmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LQ0KPiA+IG5mcyIgaW4NCj4gPiB0aGUgYm9k
eSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiA+IE1vcmUgbWFq
b3Jkb21vIGluZm8gYXTCoMKgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5o
dG1sDQo+IA0KPiANCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50
YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH v2 1/1] NFS prevent double free in async nfs4_exchange_id
  2017-03-11 15:49       ` Trond Myklebust
@ 2017-03-13 14:36         ` Olga Kornievskaia
  0 siblings, 0 replies; 6+ messages in thread
From: Olga Kornievskaia @ 2017-03-13 14:36 UTC (permalink / raw)
  To: Trond.Myklebust, anna.schumaker; +Cc: linux-nfs

Since rpc_task is async, the release function should be called which
will free the impl_id, scope, and owner.

Trond pointed at 2 more problems:
-- use of client pointer after free in the nfs4_exchangeid_release() function
-- cl_count mismatch if rpc_run_task() isn't run

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

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 59be0f7..1a65af2 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7426,11 +7426,11 @@ static void nfs4_exchange_id_release(void *data)
 	struct nfs41_exchange_id_data *cdata =
 					(struct nfs41_exchange_id_data *)data;
 
-	nfs_put_client(cdata->args.client);
 	if (cdata->xprt) {
 		xprt_put(cdata->xprt);
 		rpc_clnt_xprt_switch_put(cdata->args.client->cl_rpcclient);
 	}
+	nfs_put_client(cdata->args.client);
 	kfree(cdata->res.impl_id);
 	kfree(cdata->res.server_scope);
 	kfree(cdata->res.server_owner);
@@ -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);
@@ -7568,6 +7566,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred,
 	kfree(calldata->res.server_owner);
 out_calldata:
 	kfree(calldata);
+	nfs_put_client(clp);
 	goto out;
 }
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-13 14:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-10 19:15 [PATCH 1/1] NFS prevent double free in async nfs4_exchange_id Olga Kornievskaia
2017-03-10 20:51 ` Trond Myklebust
2017-03-10 21:35   ` Olga Kornievskaia
2017-03-10 21:56     ` Olga Kornievskaia
2017-03-11 15:49       ` Trond Myklebust
2017-03-13 14:36         ` [PATCH v2 " Olga Kornievskaia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.