What is NFSv4 READDIR doesn't return a filehandle....
Subsystems (2)
In NFSv4, the server can report which attributes it chose to return in a READDIR reply.
A customer has come across a server which does not return the filehandle information (is that allowed?).
A consequence of this is that Linux/NFS gets confused. nfs_readdir_page_filler calls nfs_prime_dcache() (because it was a readdir plus request that was sent) and nfs_prime_dcache goes ahead and creates an inode based on the filehandle that it has.
However decode_attr_filehandle() had happily decoded nothing as the FATTR4_WORD0_FILEHANDLE bit wasn't set.
So the inode gets created with a zero-length filehandle and when this gets sent back to the server to act on the inode, it gets NFS4ERR_BADHANDLE to the PUTFH op.
So should nfs_prime_dcache() abort if the filehandle doesn't exist (patch below) or should nfs_fhget() return an error if the filehandle is empty?
Or maybe this behaviour should be detected and readdir should be disabled for that server?
Suggestions?
Thanks,
NeilBrown
fs/nfs/dir.c +4 -0
@@ -442,6 +442,10 @@ void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)if (filename.len ==2 && filename.name[1 ] =='.' )return ; } +if (entry->fh.size ==0 ) +/* Server didn't return a filehandle */ +return ; + filename.hash =full_name_hash (filename.name, filename.len); dentry =d_lookup (parent, &filename);
Quoted 8 lines
-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of NeilBrown
Sent: Sunday, September 16, 2012 7:06 PM
To: [email protected]
Subject: What is NFSv4 READDIR doesn't return a filehandle....
Quoted 5 lines
In NFSv4, the server can report which attributes it chose to return in a READDIR reply.
A customer has come across a server which does not return the filehandle information (is that allowed?).
The filehandle attribute is a mandatory attribute according to RFC3530, so I believe that the answer is "no".
Quoted 10 lines
A consequence of this is that Linux/NFS gets confused.
nfs_readdir_page_fillercallsnfs_prime_dcache()(because it was a readdir plus request that was sent) andnfs_prime_dcachegoes ahead and creates an inode based on the filehandle that it has.
Howeverdecode_attr_filehandle()had happily decoded nothing as theFATTR4_WORD0_FILEHANDLEbit wasn't set.
So the inode gets created with a zero-length filehandle and when this gets sent back to the server to act on the inode, it getsNFS4ERR_BADHANDLEto the PUTFH op.
Quoted 6 lines
So should
nfs_prime_dcache()abort if the filehandle doesn't exist (patch below) or shouldnfs_fhget()return an error if the filehandle is empty?Or maybe this behaviour should be detected and readdir should be disabled for that server?
I don't want to have to code the client to deal with broken servers. If we start down that path, then we'll end up doing nothing else.
I can, however, see a case for extending the "nordirplus" mount option to cover NFSv4. Currently it only acts on NFSv3 mounts...
Cheers
Trond
Quoted 29 lines · Trond
Quoted 8 lines
-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of NeilBrown
Sent: Sunday, September 16, 2012 7:06 PM
To: [email protected]
Subject: What is NFSv4 READDIR doesn't return a filehandle....Quoted 5 lines
In NFSv4, the server can report which attributes it chose to return in a READDIR reply.
A customer has come across a server which does not return the filehandle information (is that allowed?).
The filehandle attribute is a mandatory attribute according to RFC3530, so I believe that the answer is "no".
Quoted 13 lines
A consequence of this is that Linux/NFS gets confused.
nfs_readdir_page_fillercallsnfs_prime_dcache()(because it was a readdir plus request that was sent) andnfs_prime_dcachegoes ahead and creates an inode based on the filehandle that it has.
Howeverdecode_attr_filehandle()had happily decoded nothing as theFATTR4_WORD0_FILEHANDLEbit wasn't set.
So the inode gets created with a zero-length filehandle and when this gets sent back to the server to act on the inode, it getsNFS4ERR_BADHANDLEto the PUTFH op.So should
nfs_prime_dcache()abort if the filehandle doesn't exist (patch below) or shouldnfs_fhget()return an error if the filehandle is empty?
Quoted 9 lines
Quoted 3 lines
Or maybe this behaviour should be detected and readdir should be disabled for that server?
I don't want to have to code the client to deal with broken servers. If we start down that path, then we'll end up doing nothing else.
I can, however, see a case for extending the "nordirplus" mount option to cover NFSv4. Currently it only acts on NFSv3 mounts...
Thanks Trond.
I'm happy with this position - less work for me :-)
As it happens, nordirplus *does* work for NFSv4 and customer had already found that this is a successful work around. They didn't want to have to use it though. I've pointed out that is really isn't our problem.
Just a thought: while coping with broken servers would not be a good path to follow, detecting protocol violations and reporting an error might be... should the NFS client treat a missing filehandle and a malformed reply?
Thanks,
NeilBrown
T24gVHVlLCAyMDEyLTA5LTE4IGF0IDEyOjA0ICsxMDAwLCBOZWlsQnJvd24gd3JvdGU6DQo+IE9u IE1vbiwgMTcgU2VwIDIwMTIgMTI6NTE6MzMgKzAwMDAgIk15a2xlYnVzdCwgVHJvbmQiDQo+IDxU cm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbT4gd3JvdGU6DQo+IA0KPiA+ID4gLS0tLS1PcmlnaW5h bCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IGxpbnV4LW5mcy1vd25lckB2Z2VyLmtlcm5lbC5v cmcgW21haWx0bzpsaW51eC1uZnMtDQo+ID4gPiBvd25lckB2Z2VyLmtlcm5lbC5vcmddIE9uIEJl aGFsZiBPZiBOZWlsQnJvd24NCj4gPiA+IFNlbnQ6IFN1bmRheSwgU2VwdGVtYmVyIDE2LCAyMDEy IDc6MDYgUE0NCj4gPiA+IFRvOiBsaW51eC1uZnNAdmdlci5rZXJuZWwub3JnDQo+ID4gPiBTdWJq ZWN0OiBXaGF0IGlzIE5GU3Y0IFJFQURESVIgZG9lc24ndCByZXR1cm4gYSBmaWxlaGFuZGxlLi4u Lg0KPiA+ID4gDQo+ID4gPiANCj4gPiA+IEluIE5GU3Y0LCB0aGUgc2VydmVyIGNhbiByZXBvcnQg d2hpY2ggYXR0cmlidXRlcyBpdCBjaG9zZSB0byByZXR1cm4gaW4gYQ0KPiA+ID4gUkVBRERJUiBy ZXBseS4NCj4gPiA+IA0KPiA+ID4gQSBjdXN0b21lciBoYXMgY29tZSBhY3Jvc3MgYSBzZXJ2ZXIg d2hpY2ggZG9lcyBub3QgcmV0dXJuIHRoZSBmaWxlaGFuZGxlDQo+ID4gPiBpbmZvcm1hdGlvbiAo aXMgdGhhdCBhbGxvd2VkPykuDQo+ID4gDQo+ID4gVGhlIGZpbGVoYW5kbGUgYXR0cmlidXRlIGlz IGEgbWFuZGF0b3J5IGF0dHJpYnV0ZSBhY2NvcmRpbmcgdG8gUkZDMzUzMCwgc28gSSBiZWxpZXZl IHRoYXQgdGhlIGFuc3dlciBpcyAibm8iLg0KPiA+IA0KPiA+ID4gQSBjb25zZXF1ZW5jZSBvZiB0 aGlzIGlzIHRoYXQgTGludXgvTkZTIGdldHMgY29uZnVzZWQuDQo+ID4gPiBuZnNfcmVhZGRpcl9w YWdlX2ZpbGxlciBjYWxscyBuZnNfcHJpbWVfZGNhY2hlKCkgKGJlY2F1c2UgaXQgd2FzIGEgcmVh ZGRpcg0KPiA+ID4gcGx1cyByZXF1ZXN0IHRoYXQgd2FzIHNlbnQpIGFuZCBuZnNfcHJpbWVfZGNh Y2hlIGdvZXMgYWhlYWQgYW5kIGNyZWF0ZXMNCj4gPiA+IGFuIGlub2RlIGJhc2VkIG9uIHRoZSBm aWxlaGFuZGxlIHRoYXQgaXQgaGFzLg0KPiA+ID4gSG93ZXZlciBkZWNvZGVfYXR0cl9maWxlaGFu ZGxlKCkgaGFkIGhhcHBpbHkgZGVjb2RlZCBub3RoaW5nIGFzIHRoZQ0KPiA+ID4gRkFUVFI0X1dP UkQwX0ZJTEVIQU5ETEUgYml0IHdhc24ndCBzZXQuDQo+ID4gPiBTbyB0aGUgaW5vZGUgZ2V0cyBj cmVhdGVkIHdpdGggYSB6ZXJvLWxlbmd0aCBmaWxlaGFuZGxlIGFuZCB3aGVuIHRoaXMgZ2V0cw0K PiA+ID4gc2VudCBiYWNrIHRvIHRoZSBzZXJ2ZXIgdG8gYWN0IG9uIHRoZSBpbm9kZSwgaXQgZ2V0 cyBORlM0RVJSX0JBREhBTkRMRSB0bw0KPiA+ID4gdGhlIFBVVEZIIG9wLg0KPiA+ID4gDQo+ID4g PiBTbyBzaG91bGQgbmZzX3ByaW1lX2RjYWNoZSgpIGFib3J0IGlmIHRoZSBmaWxlaGFuZGxlIGRv ZXNuJ3QgZXhpc3QgKHBhdGNoDQo+ID4gPiBiZWxvdykgb3Igc2hvdWxkIG5mc19maGdldCgpIHJl dHVybiBhbiBlcnJvciBpZiB0aGUgZmlsZWhhbmRsZSBpcyBlbXB0eT8NCj4gPiA+IA0KPiA+ID4g T3IgbWF5YmUgdGhpcyBiZWhhdmlvdXIgc2hvdWxkIGJlIGRldGVjdGVkIGFuZCByZWFkZGlyIHNo b3VsZCBiZSBkaXNhYmxlZA0KPiA+ID4gZm9yIHRoYXQgc2VydmVyPw0KPiA+ID4gDQo+ID4gDQo+ ID4gSSBkb24ndCB3YW50IHRvIGhhdmUgdG8gY29kZSB0aGUgY2xpZW50IHRvIGRlYWwgd2l0aCBi cm9rZW4gc2VydmVycy4gSWYgd2Ugc3RhcnQgZG93biB0aGF0IHBhdGgsIHRoZW4gd2UnbGwgZW5k IHVwIGRvaW5nIG5vdGhpbmcgZWxzZS4NCj4gPiANCj4gPiBJIGNhbiwgaG93ZXZlciwgc2VlIGEg Y2FzZSBmb3IgZXh0ZW5kaW5nIHRoZSAibm9yZGlycGx1cyIgbW91bnQgb3B0aW9uIHRvIGNvdmVy IE5GU3Y0LiBDdXJyZW50bHkgaXQgb25seSBhY3RzIG9uIE5GU3YzIG1vdW50cy4uLg0KPiA+IA0K PiA+DQo+IA0KPiBUaGFua3MgVHJvbmQuDQo+IEknbSBoYXBweSB3aXRoIHRoaXMgcG9zaXRpb24g LSBsZXNzIHdvcmsgZm9yIG1lIDotKQ0KPiANCj4gQXMgaXQgaGFwcGVucywgbm9yZGlycGx1cyAq ZG9lcyogd29yayBmb3IgTkZTdjQgYW5kIGN1c3RvbWVyIGhhZCBhbHJlYWR5DQo+IGZvdW5kIHRo YXQgdGhpcyBpcyBhIHN1Y2Nlc3NmdWwgd29yayBhcm91bmQuICBUaGV5IGRpZG4ndCB3YW50IHRv IGhhdmUgdG8gdXNlDQo+IGl0IHRob3VnaC4gIEkndmUgcG9pbnRlZCBvdXQgdGhhdCBpcyByZWFs bHkgaXNuJ3Qgb3VyIHByb2JsZW0uDQoNCkdvb2QhIEkgd2FzIHdvcnJpZWQgdGhhdCBub3JkaXJw bHVzIHdhc24ndCB3b3JraW5nIGZvciBORlN2NC4NCg0KSWYgdGhlcmUgaXMgYW4gZXhpc3Rpbmcg d29ya2Fyb3VuZCwgdGhlbiBJIGRvIG5vdCBhdCBhbGwgYWNjZXB0IHRoZQ0KYXJndW1lbnQgdGhh dCB3ZSBuZWVkIHRvIGFkZCBjbGllbnQtc2lkZSBwYXRjaGVzIHRvIGFjY29tbW9kYXRlDQpicm9r ZW5uZXNzIG9uIHRoZSBORlMgc2VydmVycy4NCg0KSWYgdGhlcmUgaXMgbm8gZXhpc3Rpbmcgd29y a2Fyb3VuZCwgdGhlbiBJJ20gd2lsbGluZyB0byBoZWxwIHBlb3BsZSB3aXRoDQphIHRlbXBvcmFy eSBmaXggd2hpbGUgdGhleSB3YWl0IGZvciB0aGUgc2VydmVyIHZlbmRvcnMgdG8gZ2V0IHRoZWly IGFjdA0KdG9nZXRoZXIuIEhvd2V2ZXIgdWx0aW1hdGVseSwgSSBkb24ndCBiZWxpZXZlIGluIGZp eGluZyBzZXJ2ZXIgYnVncyBvbg0KdGhlIGNsaWVudDogdGhvc2UgdGVtcG9yYXJ5IGZpeGVzIHNo b3VsZCBjZXJ0YWlubHkgbm90IGJlIGZpbmRpbmcgdGhlaXINCndheSBpbnRvIHRoZSB1cHN0cmVh bSBrZXJuZWwsIGFuZCBhcyBhIGNvbnNlcXVlbmNlIHRoZXkgc2hvdWxkIHByb2JhYmx5DQpub3Qg Z28gaW50byBkaXN0cmlidXRpb24ga2VybmVscyBlaXRoZXIgKGFsdGhvdWdoIHRoYXQgaXMgeW91 ciBjYWxsLCBhbmQNCm5vdCBtaW5lIDotKSkuDQoNCj4gSnVzdCBhIHRob3VnaHQ6IHdoaWxlIGNv cGluZyB3aXRoIGJyb2tlbiBzZXJ2ZXJzIHdvdWxkIG5vdCBiZSBhIGdvb2QgcGF0aCB0bw0KPiBm b2xsb3csIGRldGVjdGluZyBwcm90b2NvbCB2aW9sYXRpb25zIGFuZCByZXBvcnRpbmcgYW4gZXJy b3IgbWlnaHQgYmUuLi4NCj4gc2hvdWxkIHRoZSBORlMgY2xpZW50IHRyZWF0IGEgbWlzc2luZyBm aWxlaGFuZGxlIGFuZCBhIG1hbGZvcm1lZCByZXBseT8NCg0KTXkgY29uY2VybiBpcyB0aGF0IHRo ZSBjbGllbnQgY2FuJ3Qgb2JqZWN0aXZlbHkganVkZ2Ugd2hhdCBjb25zdGl0dXRlcyBhDQp2YWxp ZCBmaWxlaGFuZGxlIGFuZCB3aGF0IGRvZXMgbm90IHVudGlsIGl0IHRyaWVzIHRvIHVzZSBpdCBp biBhbiBSUEMNCmNhbGwuDQpHaXZlbiB0aGF0IHByZW1pc2UsIGl0IG1ha2VzIG1vcmUgc2Vuc2Ug dG8gY29uY2VudHJhdGUgb24gaGFuZGxpbmcgdGhlDQpjYXNlcyB3aGVyZSB0aGUgdXNhZ2UgZmFp bHMuIEplZmYgTGF5dG9uJ3MgdmZzIEVTVEFMRSBwYXRjaGVzIGFyZSBhIGdvb2QNCmNhc2UgaW4g cG9pbnQuDQoNCkNoZWVycw0KICBUcm9uZA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXgg TkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0QG5ldGFwcC5j b20NCnd3dy5uZXRhcHAuY29tDQo=
Quoted 68 lines · Trond
Quoted 42 lines · NeilBrown
Quoted 29 lines · Trond
Quoted 8 lines
-----Original Message-----
From: [email protected] [mailto:linux-nfs- [email protected]] On Behalf Of NeilBrown
Sent: Sunday, September 16, 2012 7:06 PM
To: [email protected]
Subject: What is NFSv4 READDIR doesn't return a filehandle....Quoted 5 lines
In NFSv4, the server can report which attributes it chose to return in a READDIR reply.
A customer has come across a server which does not return the filehandle information (is that allowed?).
The filehandle attribute is a mandatory attribute according to RFC3530, so I believe that the answer is "no".
Quoted 13 lines
A consequence of this is that Linux/NFS gets confused.
nfs_readdir_page_fillercallsnfs_prime_dcache()(because it was a readdir plus request that was sent) andnfs_prime_dcachegoes ahead and creates an inode based on the filehandle that it has.
Howeverdecode_attr_filehandle()had happily decoded nothing as theFATTR4_WORD0_FILEHANDLEbit wasn't set.
So the inode gets created with a zero-length filehandle and when this gets sent back to the server to act on the inode, it getsNFS4ERR_BADHANDLEto the PUTFH op.So should
nfs_prime_dcache()abort if the filehandle doesn't exist (patch below) or shouldnfs_fhget()return an error if the filehandle is empty?Quoted 9 lines
Quoted 3 lines
Or maybe this behaviour should be detected and readdir should be disabled for that server?
I don't want to have to code the client to deal with broken servers. If we start down that path, then we'll end up doing nothing else.
I can, however, see a case for extending the "nordirplus" mount option to cover NFSv4. Currently it only acts on NFSv3 mounts...
Quoted 6 lines
Thanks Trond.
I'm happy with this position - less work for me :-)As it happens, nordirplus *does* work for NFSv4 and customer had already found that this is a successful work around. They didn't want to have to use it though. I've pointed out that is really isn't our problem.
Good! I was worried that nordirplus wasn't working for NFSv4.
If there is an existing workaround, then I do not at all accept the argument that we need to add client-side patches to accommodate brokenness on the NFS servers.
If there is no existing workaround, then I'm willing to help people with a temporary fix while they wait for the server vendors to get their act together. However ultimately, I don't believe in fixing server bugs on the client: those temporary fixes should certainly not be finding their way into the upstream kernel, and as a consequence they should probably not go into distribution kernels either (although that is your call, and not mine :-)).
Quoted 3 lines
Just a thought: while coping with broken servers would not be a good path to follow, detecting protocol violations and reporting an error might be... should the NFS client treat a missing filehandle and a malformed reply?
Quoted 7 lines
My concern is that the client can't objectively judge what constitutes a valid filehandle and what does not until it tries to use it in an RPC call.
Given that premise, it makes more sense to concentrate on handling the cases where the usage fails. Jeff Layton's vfs ESTALE patches are a good case in point.
Wait though -- is it not safe to assume that a zero length filehandle is invalid?
Neil's earlier patch checked for entry->fh.size == 0, would it not be reasonable to warn once per server when we get back such a fh?
Signature · 1 line
Jeff Layton <[email protected]>
T24gU3VuLCAyMDEyLTA5LTIzIGF0IDIwOjQxIC0wNDAwLCBKZWZmIExheXRvbiB3cm90ZToNCj4g V2FpdCB0aG91Z2ggLS0gaXMgaXQgbm90IHNhZmUgdG8gYXNzdW1lIHRoYXQgYSB6ZXJvIGxlbmd0 aCBmaWxlaGFuZGxlDQo+IGlzIGludmFsaWQ/DQo+IA0KPiBOZWlsJ3MgZWFybGllciBwYXRjaCBj aGVja2VkIGZvciBlbnRyeS0+Zmguc2l6ZSA9PSAwLCB3b3VsZCBpdCBub3QgYmUNCj4gcmVhc29u YWJsZSB0byB3YXJuIG9uY2UgcGVyIHNlcnZlciB3aGVuIHdlIGdldCBiYWNrIHN1Y2ggYSBmaD8N Cg0KVGhlIHBvaW50IGlzIHRoYXQgc2FuaXR5IGNoZWNraW5nIG9mIGZpbGVoYW5kbGVzIGJ5IHRo ZSBjbGllbnQgaXMgYQ0KdG90YWxseSBfaW5zYW5lXyBjb25jZXB0LiBZZXMgd2UgY2FuIGRvIGl0 IGZvciBhIGNvdXBsZSBvZiBjb3JuZXIgY2FzZXMuDQpObywgd2Ugc3VyZSBhcyBoZWxsIGRvIE5P VCB3YW50IHRvIGdldCBzdHVjayBkb2luZyBpdCBmb3IgdGhlIGdlbmVyYWwNCmNhc2UuDQoNClRy b25kDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXIN Cg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg==