NFS prevent double free in async nfs4_exchange_id
Subsystems (2)
PATCH 1/1 NFS prevent double free in async nfs4_exchange_id
Since rpc_task is async, the release function should be called which will free the impl_id, scope, and owner.
--- fs/nfs/nfs4proc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
fs/nfs/nfs4proc.c +2 -4
@@ -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);
Signature · 1 line
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=
PATCH 1/1 NFS prevent double free in async nfs4_exchange_id
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
--- fs/nfs/nfs4proc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
fs/nfs/nfs4proc.c +5 -5
@@ -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:
Signature · 1 line
1.8.3.1
Quoted 44 lines · Olga Kornievskaia
Since
rpc_taskis async, the release function should be called which will free theimpl_id, scope, and owner.Trond pointed at 2 more problems:
-- use of client pointer after free in thenfs4_exchangeid_release()function --cl_countmismatch ifrpc_run_task()isn't runFixes 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(-)fs/nfs/nfs4proc.c +3
-5@@ -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);
fs/nfs/nfs4proc.c static int _nfs4_proc_exchange_id(struct nfs_client *clp, struct rpc_cred *cred, · line 7563
Quoted 8 lines
fs/nfs/nfs4proc.c +2
-0@@ -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.
Quoted 7 lines
--
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=
PATCH v2 1/1 NFS prevent double free in async nfs4_exchange_id
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
--- fs/nfs/nfs4proc.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
fs/nfs/nfs4proc.c +4 -5
@@ -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; }
Signature · 1 line
1.8.3.1