2018-02-21 18:46:48

by Bill Baker

[permalink] [raw]
Subject: [PATCH v2] nfs: system crashes after NFS4ERR_MOVED recovery


nfs4_update_server unconditionally releases the nfs_client for the
source server. If migration fails, this can cause the source server's
nfs_client struct to be left with a low reference count, resulting in
use-after-free. Also, adjust reference count handling for ELOOP.

NFS: state manager: migration failed on NFSv4 server nfsvmu10 with error 6
WARNING: CPU: 16 PID: 17960 at fs/nfs/client.c:281 nfs_put_client+0xfa/0x110 [nfs]()
nfs_put_client+0xfa/0x110 [nfs]
nfs4_run_state_manager+0x30/0x40 [nfsv4]
kthread+0xd8/0xf0

BUG: unable to handle kernel NULL pointer dereference at 00000000000002a8
nfs4_xdr_enc_write+0x6b/0x160 [nfsv4]
rpcauth_wrap_req+0xac/0xf0 [sunrpc]
call_transmit+0x18c/0x2c0 [sunrpc]
__rpc_execute+0xa6/0x490 [sunrpc]
rpc_async_schedule+0x15/0x20 [sunrpc]
process_one_work+0x160/0x470
worker_thread+0x112/0x540
? rescuer_thread+0x3f0/0x3f0
kthread+0xd8/0xf0

This bug was introduced by 32e62b7c ("NFS: Add nfs4_update_server"),
but the fix applies cleanly to 52442f9b ("NFS4: Avoid migration loops")

Reported-by: Helen Chao <[email protected]>
Fixes: 52442f9b11b7 ("NFS4: Avoid migration loops")
Signed-off-by: Bill Baker <[email protected]>
Reviewed-by: Chuck Lever <[email protected]>
---
fs/nfs/nfs4client.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

changes since v1:
ensure clp isn't leaked if source and dest servers are the same

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 65a7e5d..2c9a9b0 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -867,8 +867,10 @@ static int nfs4_set_client(struct nfs_server *server,
if (IS_ERR(clp))
return PTR_ERR(clp);

- if (server->nfs_client == clp)
+ if (server->nfs_client == clp) {
+ nfs_put_client(clp);
return -ELOOP;
+ }

/*
* Query for the lease time on clientid setup or renewal
@@ -1226,11 +1228,11 @@ int nfs4_update_server(struct nfs_server *server, const char *hostname,
clp->cl_proto, clnt->cl_timeout,
clp->cl_minorversion, net);
clear_bit(NFS_MIG_TSM_POSSIBLE, &server->mig_status);
- nfs_put_client(clp);
if (error != 0) {
nfs_server_insert_lists(server);
return error;
}
+ nfs_put_client(clp);

if (server->nfs_client->cl_hostname == NULL)
server->nfs_client->cl_hostname = kstrdup(hostname, GFP_KERNEL);
--
1.8.3.1



2018-02-22 17:20:26

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH v2] nfs: system crashes after NFS4ERR_MOVED recovery

T24gV2VkLCAyMDE4LTAyLTIxIGF0IDEyOjQ2IC0wNjAwLCBCaWxsLkJha2VyQG9yYWNsZS5jb20g
d3JvdGU6DQo+IG5mczRfdXBkYXRlX3NlcnZlciB1bmNvbmRpdGlvbmFsbHkgcmVsZWFzZXMgdGhl
IG5mc19jbGllbnQgZm9yIHRoZQ0KPiBzb3VyY2Ugc2VydmVyLiBJZiBtaWdyYXRpb24gZmFpbHMs
IHRoaXMgY2FuIGNhdXNlIHRoZSBzb3VyY2Ugc2VydmVyJ3MNCj4gbmZzX2NsaWVudCBzdHJ1Y3Qg
dG8gYmUgbGVmdCB3aXRoIGEgbG93IHJlZmVyZW5jZSBjb3VudCwgcmVzdWx0aW5nIGluDQo+IHVz
ZS1hZnRlci1mcmVlLiAgQWxzbywgYWRqdXN0IHJlZmVyZW5jZSBjb3VudCBoYW5kbGluZyBmb3Ig
RUxPT1AuDQo+IA0KPiBORlM6IHN0YXRlIG1hbmFnZXI6IG1pZ3JhdGlvbiBmYWlsZWQgb24gTkZT
djQgc2VydmVyIG5mc3ZtdTEwIHdpdGgNCj4gZXJyb3IgNg0KPiBXQVJOSU5HOiBDUFU6IDE2IFBJ
RDogMTc5NjAgYXQgZnMvbmZzL2NsaWVudC5jOjI4MQ0KPiBuZnNfcHV0X2NsaWVudCsweGZhLzB4
MTEwIFtuZnNdKCkNCj4gCW5mc19wdXRfY2xpZW50KzB4ZmEvMHgxMTAgW25mc10NCj4gCW5mczRf
cnVuX3N0YXRlX21hbmFnZXIrMHgzMC8weDQwIFtuZnN2NF0NCj4gCWt0aHJlYWQrMHhkOC8weGYw
DQo+IA0KPiBCVUc6IHVuYWJsZSB0byBoYW5kbGUga2VybmVsIE5VTEwgcG9pbnRlciBkZXJlZmVy
ZW5jZSBhdA0KPiAwMDAwMDAwMDAwMDAwMmE4DQo+IAluZnM0X3hkcl9lbmNfd3JpdGUrMHg2Yi8w
eDE2MCBbbmZzdjRdDQo+IAlycGNhdXRoX3dyYXBfcmVxKzB4YWMvMHhmMCBbc3VucnBjXQ0KPiAJ
Y2FsbF90cmFuc21pdCsweDE4Yy8weDJjMCBbc3VucnBjXQ0KPiAJX19ycGNfZXhlY3V0ZSsweGE2
LzB4NDkwIFtzdW5ycGNdDQo+IAlycGNfYXN5bmNfc2NoZWR1bGUrMHgxNS8weDIwIFtzdW5ycGNd
DQo+IAlwcm9jZXNzX29uZV93b3JrKzB4MTYwLzB4NDcwDQo+IAl3b3JrZXJfdGhyZWFkKzB4MTEy
LzB4NTQwDQo+IAk/IHJlc2N1ZXJfdGhyZWFkKzB4M2YwLzB4M2YwDQo+IAlrdGhyZWFkKzB4ZDgv
MHhmMA0KPiANCj4gVGhpcyBidWcgd2FzIGludHJvZHVjZWQgYnkgMzJlNjJiN2MgKCJORlM6IEFk
ZCBuZnM0X3VwZGF0ZV9zZXJ2ZXIiKSwNCj4gYnV0IHRoZSBmaXggYXBwbGllcyBjbGVhbmx5IHRv
IDUyNDQyZjliICgiTkZTNDogQXZvaWQgbWlncmF0aW9uDQo+IGxvb3BzIikNCj4gDQo+IFJlcG9y
dGVkLWJ5OiBIZWxlbiBDaGFvIDxoZWxlbi5jaGFvQG9yYWNsZS5jb20+DQo+IEZpeGVzOiA1MjQ0
MmY5YjExYjcgKCJORlM0OiBBdm9pZCBtaWdyYXRpb24gbG9vcHMiKQ0KPiBTaWduZWQtb2ZmLWJ5
OiBCaWxsIEJha2VyIDxiaWxsLmJha2VyQG9yYWNsZS5jb20+DQo+IFJldmlld2VkLWJ5OiBDaHVj
ayBMZXZlciA8Y2h1Y2subGV2ZXJAb3JhY2xlLmNvbT4NCj4gLS0tDQo+ICBmcy9uZnMvbmZzNGNs
aWVudC5jIHwgNiArKysrLS0NCj4gIDEgZmlsZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDIg
ZGVsZXRpb25zKC0pDQo+IA0KPiBjaGFuZ2VzIHNpbmNlIHYxOg0KPiAJZW5zdXJlIGNscCBpc24n
dCBsZWFrZWQgaWYgc291cmNlIGFuZCBkZXN0IHNlcnZlcnMgYXJlIHRoZSBzYW1lDQo+IA0KPiBk
aWZmIC0tZ2l0IGEvZnMvbmZzL25mczRjbGllbnQuYyBiL2ZzL25mcy9uZnM0Y2xpZW50LmMNCj4g
aW5kZXggNjVhN2U1ZC4uMmM5YTliMCAxMDA2NDQNCj4gLS0tIGEvZnMvbmZzL25mczRjbGllbnQu
Yw0KPiArKysgYi9mcy9uZnMvbmZzNGNsaWVudC5jDQo+IEBAIC04NjcsOCArODY3LDEwIEBAIHN0
YXRpYyBpbnQgbmZzNF9zZXRfY2xpZW50KHN0cnVjdCBuZnNfc2VydmVyDQo+ICpzZXJ2ZXIsDQo+
ICAJaWYgKElTX0VSUihjbHApKQ0KPiAgCQlyZXR1cm4gUFRSX0VSUihjbHApOw0KPiAgDQo+IC0J
aWYgKHNlcnZlci0+bmZzX2NsaWVudCA9PSBjbHApDQo+ICsJaWYgKHNlcnZlci0+bmZzX2NsaWVu
dCA9PSBjbHApIHsNCj4gKwkJbmZzX3B1dF9jbGllbnQoY2xwKTsNCj4gIAkJcmV0dXJuIC1FTE9P
UDsNCj4gKwl9DQo+ICANCj4gIAkvKg0KPiAgCSAqIFF1ZXJ5IGZvciB0aGUgbGVhc2UgdGltZSBv
biBjbGllbnRpZCBzZXR1cCBvciByZW5ld2FsDQo+IEBAIC0xMjI2LDExICsxMjI4LDExIEBAIGlu
dCBuZnM0X3VwZGF0ZV9zZXJ2ZXIoc3RydWN0IG5mc19zZXJ2ZXINCj4gKnNlcnZlciwgY29uc3Qg
Y2hhciAqaG9zdG5hbWUsDQo+ICAJCQkJY2xwLT5jbF9wcm90bywgY2xudC0+Y2xfdGltZW91dCwN
Cj4gIAkJCQljbHAtPmNsX21pbm9ydmVyc2lvbiwgbmV0KTsNCj4gIAljbGVhcl9iaXQoTkZTX01J
R19UU01fUE9TU0lCTEUsICZzZXJ2ZXItPm1pZ19zdGF0dXMpOw0KPiAtCW5mc19wdXRfY2xpZW50
KGNscCk7DQo+ICAJaWYgKGVycm9yICE9IDApIHsNCj4gIAkJbmZzX3NlcnZlcl9pbnNlcnRfbGlz
dHMoc2VydmVyKTsNCj4gIAkJcmV0dXJuIGVycm9yOw0KPiAgCX0NCj4gKwluZnNfcHV0X2NsaWVu
dChjbHApOw0KPiAgDQo+ICAJaWYgKHNlcnZlci0+bmZzX2NsaWVudC0+Y2xfaG9zdG5hbWUgPT0g
TlVMTCkNCj4gIAkJc2VydmVyLT5uZnNfY2xpZW50LT5jbF9ob3N0bmFtZSA9IGtzdHJkdXAoaG9z
dG5hbWUsDQo+IEdGUF9LRVJORUwpOw0KDQpUaGFua3MgQmlsbCEgQXBwbGllZCB0byBsaW51eC1u
ZXh0Li4uDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWlu
ZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRhdGEuY29tDQo=