| Message ID | 0A7143CE-0592-444D-BA53-9CBB33E4373F@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
DQo+IE9uIE9jdCAzMCwgMjAxNiwgYXQgMjE6MTgsIFZpdGFsaXkgR3VzZXYgPGd1c2V2LnZpdGFs aXlAZ21haWwuY29tPiB3cm90ZToNCj4gDQo+IEhpLg0KPiANCj4gRHVyaW5nIHNvbWUgdGVzdHMg SeKAmXZlIHNlZW4gbmZzLWNsaWVudCBjcmFzaGVzLiBJdCB3YXMganVzdCByZWFkaW5nIGZpbGUg dmlhIE5GU3Y0LjEgcHJvdG9jb2wuIA0KPiANCj4gVGhlIHBhbmljIG1lc3NhZ2UgaXMgYmVsb3cs ICBmaXhpbmcgcGF0Y2ggaXMgYXR0YWNoZWQuDQoNCldoeSBkb2VzIHRoaXMgbmVlZCB0byBiZSBm aXhlZCBvbiB0aGUgY2xpZW50PyBJdCBsb29rcyBsaWtlIGEgc2VydmVyIGJ1Z+KApiBJbiBhbnkg Y2FzZSwgdGhlIGZpeCB5b3UgcHJvcG9zZSBpcyBnb2luZyB0byBsZWF2ZSB0aGUgY2xpZW50IHdp dGggYnJva2VuIG9wZW4gc3RhdGUuDQoNCj4gDQo+IOKAlOKAlOKAlA0KPiAiQlVHOiB1bmFibGUg dG8gaGFuZGxlIGtlcm5lbCBOVUxMIHBvaW50ZXIgZGVyZWZlcmVuY2UgYXQgMDAwMDAwMDAwMDAw MDA5MA0KPiAiSVA6IFs8ZmZmZmZmZmZiMDQ5ZWVmYz5dIF9yYXdfc3Bpbl9sb2NrKzB4Yy8weDMw DQo+IFBHRCAyYTFjMDY3IFBVRCAyOWU1MDY3IFBNRCAwDQo+IE9vcHM6IDAwMDIgWyMxXSBTTVAN Cj4gQ1BVOiAxIFBJRDogMzU3MyBDb21tOiBrd29ya2VyLzE6MCBOb3QgdGFpbnRlZCA0LjguMC0y Ni1nZW5lcmljICMyOC1VYnVudHUNCj4gSGFyZHdhcmUgbmFtZTogVk13YXJlLCBJbmMuIFZNd2Fy ZSBWaXJ0dWFsIFBsYXRmb3JtLzQ0MEJYIERlc2t0b3AgUmVmZXJlbmNlIFBsYXRmb3JtLCBCSU9T IDYuMDAgMDcvMDIvMjAxNQ0KPiBXb3JrcXVldWU6IHJwY2lvZCBycGNfYXN5bmNfc2NoZWR1bGUg W3N1bnJwY10NCj4gdGFzazogZmZmZmEwMTdjYjUzNDc0MCB0YXNrLnN0YWNrOiBmZmZmYTAxNzgy YjQ0MDAwDQo+IFJJUDogMDAxMDpbPGZmZmZmZmZmYjA0OWVlZmM+XSAgWzxmZmZmZmZmZmIwNDll ZWZjPl0gX3Jhd19zcGluX2xvY2srMHhjLzB4MzBOXg0KPiBSU1A6IDAwMTg6ZmZmZmEwMTc4MmI0 N2Q0OCAgRUZMQUdTOiAwMDAxMDI0Ng0KPiBSQVg6IDAwMDAwMDAwMDAwMDAwMDAgUkJYOiBmZmZm YTAxN2M3MzVkMjA4IFJDWDogZmZmZmEwMTc4MzgxNmMwMA0KPiBSRFg6IDAwMDAwMDAwMDAwMDAw MDEgUlNJOiBmZmZmYTAxN2M3MzVkMWUwIFJESTogMDAwMDAwMDAwMDAwMDA5MA0KPiBSQlA6IGZm ZmZhMDE3ODJiNDdkNzggUjA4OiBmZmZmYTAxN2NlYzU4YTAwIFIwOTogMDAwMDAwMDAwMDAwMDAw MA0KPiBSMTA6IDAwMDAwMDAwMDAwMDAwMDAgUjExOiAwMDAwMDBiNDA5OGVkMDM2IFIxMjogZmZm ZmEwMTc4MzgxNmMwMA0KPiBSMTM6IDAwMDAwMDAwMDAwMDAwMDAgUjE0OiAwMDAwMDAwMDAwMDAw MDkwIFIxNTogZmZmZmEwMTdjNzM1ZDFlMA0KPiBGUzogIDAwMDAwMDAwMDAwMDAwMDAoMDAwMCkg R1M6ZmZmZmEwMTdjZWM0MDAwMCgwMDAwKSBrbmxHUzowMDAwMDAwMDAwMDAwMDAwDQo+IENTOiAg MDAxMCBEUzogMDAwMCBFUzogMDAwMCBDUjA6IDAwMDAwMDAwODAwNTAwMzMNCj4gQ1IyOiAwMDAw MDAwMDAwMDAwMDkwIENSMzogMDAwMDAwMDAwMmEwYzAwMCBDUjQ6IDAwMDAwMDAwMDAxNDA2ZTAN Cj4gU3RhY2s6DQo+IGZmZmZmZmZmYzA4MzYyMDggZmZmZmEwMTc4MzgxNmMwMCBmZmZmZmZmZmMw NGM5MDcwIGZmZmZmZmZmYzA0YzkwNzANCj4gMDAwMDAwMDAwMDAwMDAwMCBmZmZmYTAxNzgzODE2 YzQwIGZmZmZhMDE3ODJiNDdkODggZmZmZmZmZmZjMDgzNjJkMA0KPiBmZmZmYTAxNzgyYjQ3ZDk4 IGZmZmZmZmZmYzA0YzkwODkgZmZmZmEwMTc4MmI0N2UwMCBmZmZmZmZmZmMwNGNiNmZkDQo+IA0K PiBDYWxsIFRyYWNlOg0KPiBbPGZmZmZmZmZmYzA4MzYyMDg+XSA/IG5mczQwX3NldHVwX3NlcXVl bmNlKzB4NDgvMHhlMCBbbmZzdjRdDQo+IFs8ZmZmZmZmZmZjMDgzNjJkMD5dIG5mczRfb3Blbl9j b25maXJtX3ByZXBhcmUrMHgzMC8weDQwIFtuZnN2NF0NCj4gWzxmZmZmZmZmZmMwNGM5MDg5Pl0g cnBjX3ByZXBhcmVfdGFzaysweDE5LzB4MjAgW3N1bnJwY10NCj4gWzxmZmZmZmZmZmMwNGNiNmZk Pl0gX19ycGNfZXhlY3V0ZSsweDhkLzB4NDIwIFtzdW5ycGNdDQo+IFs8ZmZmZmZmZmZjMDRjYmFh Mj5dIHJwY19hc3luY19zY2hlZHVsZSsweDEyLzB4MjAgW3N1bnJwY10NCj4gWzxmZmZmZmZmZmFm YzlkNjFjPl0gcHJvY2Vzc19vbmVfd29yaysweDFmYy8weDRiMA0KPiBbPGZmZmZmZmZmYWZjOWQ5 MWI+XSB3b3JrZXJfdGhyZWFkKzB4NGIvMHg1MDANCj4gWzxmZmZmZmZmZmFmY2EzYzE4Pl0ga3Ro cmVhZCsweGQ4LzB4ZjANCj4gWzxmZmZmZmZmZmIwNDlmMjlmPl0gcmV0X2Zyb21fZm9yaysweDFm LzB4NDANCj4gWzxmZmZmZmZmZmFmY2EzYjQwPl0gPyBrdGhyZWFkX2NyZWF0ZV9vbl9ub2RlKzB4 MWUwLzB4MWUwDQo+IENvZGU6IDAwIDAxIDAwIDAwIGYwIDBmIGMxIDM3IDgxIGM2IDAwIDAxIDAw IDAwIDQwIDg0IGY2IDc1IDAxIGMzIDU1IDQ4IDg5IGU1IGU4IGUyIDE5IDgzIGZmIDVkIGMzIDBm IDFmIDQ0IDAwIDAwIDMxIGMwIGJhIDAxIDAwIDAwIDAwIDxmMD4gMGYgYjEgMTcgODUgYzAgNzUg MDEgYzMgNTUgODkgYzYgNDggODkgZTUgZTggYzAgMDEgODMgZmYgNjYgDQo+ICJSSVAgIFs8ZmZm ZmZmZmZiMDQ5ZWVmYz5dIF9yYXdfc3Bpbl9sb2NrKzB4Yy8weDMwDQo+IA0KPiDigJTigJTigJQN Cg0K -- 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
> On 31 Oct 2016, at 18:46, Trond Myklebust <trondmy@primarydata.com> wrote: > > >> On Oct 30, 2016, at 21:18, Vitaliy Gusev <gusev.vitaliy@gmail.com> wrote: >> >> Hi. >> >> During some tests I’ve seen nfs-client crashes. It was just reading file via NFSv4.1 protocol. >> >> The panic message is below, fixing patch is attached. > > Why does this need to be fixed on the client? It looks like a server bug… In any case, the fix you propose is going to leave the client with broken open state. Do you like to get crash every time a remote side sends improper datas? I believe not. I proposed just ignore the flag OPEN4_RESULT_CONFIRM for nfs4.1+ clients. RFC5661 has description that allows a client to do that: o OPEN4_RESULT_CONFIRM is deprecated and MUST NOT be returned by an NFSv4.1 server. ——— Thanks, Vitaliy Gusev -- 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
> On Oct 31, 2016, at 13:31, Vitaliy Gusev <gusev.vitaliy@gmail.com> wrote: > > >> On 31 Oct 2016, at 18:46, Trond Myklebust <trondmy@primarydata.com> wrote: >> >> >>> On Oct 30, 2016, at 21:18, Vitaliy Gusev <gusev.vitaliy@gmail.com> wrote: >>> >>> Hi. >>> >>> During some tests I’ve seen nfs-client crashes. It was just reading file via NFSv4.1 protocol. >>> >>> The panic message is below, fixing patch is attached. >> >> Why does this need to be fixed on the client? It looks like a server bug… In any case, the fix you propose is going to leave the client with broken open state. > > Do you like to get crash every time a remote side sends improper datas? I believe not. There are a million other ways to screw a client over if your server is buggy or compromised. > > I proposed just ignore the flag OPEN4_RESULT_CONFIRM for nfs4.1+ clients. > RFC5661 has description that allows a client to do that: > > o OPEN4_RESULT_CONFIRM is deprecated and MUST NOT be returned by an > NFSv4.1 server. I know what the spec says. The point is that the client will leak memory, and fail to handle the situation correctly when the server returns OPEN4_RESULT_CONFIRM with or with the patch that you are proposing. The right thing to do here would rather be to print out a big fat warning to the user, and then possibly also to kill the mount.
> On 31 Oct 2016, at 20:54, Trond Myklebust <trondmy@primarydata.com> wrote: > >> >> On Oct 31, 2016, at 13:31, Vitaliy Gusev <gusev.vitaliy@gmail.com> wrote: >> Do you like to get crash every time a remote side sends improper datas? I believe not. > > There are a million other ways to screw a client over if your server is buggy or compromised. I agree, but crashes are not right way to work with incorrect datas and that’s why I reported problem. >> >> I proposed just ignore the flag OPEN4_RESULT_CONFIRM for nfs4.1+ clients. >> RFC5661 has description that allows a client to do that: >> >> o OPEN4_RESULT_CONFIRM is deprecated and MUST NOT be returned by an >> NFSv4.1 server. > > I know what the spec says. The point is that the client will leak memory, and fail to handle the situation correctly when the server returns OPEN4_RESULT_CONFIRM with or with the patch that you are proposing. > The right thing to do here would rather be to print out a big fat warning to the user, and then possibly also to kill the mount. That’s a good point. What if return error for OPEN operation instead of kill the mount? ——— Thanks, Vitaliy Gusev --- Vitaliy Gusev -- 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
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index a9dec32..054ce67 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2023,6 +2023,9 @@ static int _nfs4_proc_open_confirm(struct nfs4_opendata *data) }; int status; + if (server->nfs_client->cl_mvops->minor_version != 0) + return 0; + nfs4_init_sequence(&data->c_arg.seq_args, &data->c_res.seq_res, 1); kref_get(&data->kref); data->rpc_done = 0;