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 <[email protected]>
---
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
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=
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 <[email protected]>
---
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
On Fri, Mar 10, 2017 at 4:35 PM, Olga Kornievskaia <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
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=
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 <[email protected]>
---
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